Skip to content

docs(analytics): record assistant review cadence and script hardening - #2348

Merged
GigaHierz merged 2 commits into
mainfrom
GigaHierz/2307-assistant-telemetry-review
Oct 2, 2026
Merged

GigaHierz merged 2 commits into
mainfrom
GigaHierz/2307-assistant-telemetry-review

Conversation

@GigaHierz

@GigaHierz GigaHierz commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Refs #2307, Refs #2302

What changed

Boxes still open

Verification

$ node --check assistant.js   # ok
$ mint broken-links
success no broken links found

🤖 Generated with Claude Code

…, load widget with crossOrigin

Refs #2307, #2302

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

@palango palango left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The crossOrigin change looks good. widget.js returns access-control-allow-origin: *, and docs-ai-assistant's next.config.ts sets it on purpose, so it won't vanish by accident. A few lines in the new ANALYTICS.md text don't match what the weekly report does, and since ANALYTICS.md is served publicly at docs.celo.org/ANALYTICS, I'd fix those before merge.

The main one is the rule against pasting raw questions into issues. The weekly report bot pastes up to 25 raw questions into a public issue every Monday (see #2343, which also includes an injection prompt the probe filter missed). The doc and the bot need to agree: either raw text in bot-filed issues is acceptable and the rule should say so, or the report should change. Details inline.

Separately, four weekly issues (#2314, #2320, #2324, #2343) are still open. The weekly issue already filters to answered: false, so a monthly Redis read mostly repeats it. It may be simpler to make triaging those issues the owned task. Also, the report doesn't skip refused: true entries, so off-topic refusals show up in the weekly gaps list. That's a fix for the assistant repo, not this PR.

Comment thread ANALYTICS.md Outdated

### Reviewing unanswered questions

- **Weekly:** an automated report files a "Docs assistant: N unanswered questions" issue listing questions that returned no citation, with injection and enumeration probes counted but not listed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The report does list some probes: route.ts passes probeClusters.slice(0, 3) (first 60 chars each) into a "Sampled rather than listed in full" block, and #2343 shows three. Suggest "with injection and enumeration probes counted and only a short sample shown".

Comment thread ANALYTICS.md Outdated

- **Weekly:** an automated report files a "Docs assistant: N unanswered questions" issue listing questions that returned no citation, with injection and enumeration probes counted but not listed.
- **Monthly:** `@celo-org/devrel` reads the Redis list `docs-assistant:questions`, filters to `answered: false`, and turns real gaps into doc issues. Questions that were refused as off-topic carry `refused: true` and are not gaps.
- The list holds raw question text. Do not paste entries into issues or PRs; describe the topic instead.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conflicts with the weekly report two lines up, which pastes up to 25 raw questions into a public issue (#2314, #2320, #2324, #2343). Which one is the policy? If bot-filed issues are fine, scope this rule to manual copies. If not, the report needs changing in docs-ai-assistant, and this PR should point to that follow-up.

Comment thread ANALYTICS.md Outdated

### Script hardening

`assistant.js` loads `https://docs-assistant.celo.org/widget.js` with `crossOrigin = 'anonymous'`; the host sends `access-control-allow-origin: *`, so the load works under CORS. There is **no Subresource Integrity hash and no Content-Security-Policy** on this site: an `integrity` hash would break the widget on every widget deploy, and the Mintlify Starter plan offers no header configuration we have confirmed. Whoever controls `docs-assistant.celo.org` can therefore run script on every docs page, so changes to that host need the same review as a change to this repo (#2302).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: name where the header comes from (docs-ai-assistant next.config.ts, the /widget.js headers rule), so whoever edits that file knows docs.celo.org now depends on it.

Comment thread ANALYTICS.md Outdated
GA4's built-in "AI Assistant" channel recognizes only ChatGPT, Gemini, DeepSeek, Copilot and Grok — not Claude or Perplexity. Known limit: a large share of AI-referred sessions arrive with no referrer and land in Direct; this channel measures the floor, not the total.
- [ ] **Custom dimensions** (event-scoped): `percent_scrolled`, `link_domain`, `ai_target`, `network`, `result`, `is_automated`, plus the assistant's `answered`, `escalated`, `truncated`, `from_api`, `status` and `href`. Without these registered the assistant events still arrive, but their parameters cannot be used in any report.
- [ ] **Before the swap: the assistant events need a path that does not go through `window.gtag`.** `widget.js` calls `track()` only `if (typeof window.gtag === 'function')`. On the live site that global is a function, provided by `integrations.ga4`; on a page carrying only the GTM container it is `undefined`. Defining `gtag(){ dataLayer.push(arguments) }` by hand does *not* rescue it — no hit goes out. So all seven `assistant_*` events stop the moment the swap merges. **Two things have to be in place before it, not one**: the widget has to push to `dataLayer` directly (#2307), *and* the container needs the `^assistant_` trigger and tag from Runbook 1 to forward those pushes. Either one alone leaves the events dark.
- [ ] **After the swap, confirm on a published page** that the assistant still reports: in the browser console `typeof window.gtag === 'function'` must be `true` (or the widget must push to `dataLayer`), then ask the assistant one question and see one `assistant_question` event in GA4 Realtime with `answered` populated.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bullet above says a hand-defined gtag shim sends no hit, so typeof window.gtag === 'function' passing doesn't show anything reaches GA4. Under GTM alone it's undefined, and widget.js still only tracks through window.gtag. I'd drop the gtag clause and keep the Realtime assistant_question event as the pass criterion. #2307 AC 2 still has the old gtag wording and needs the same update.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@GigaHierz

Copy link
Copy Markdown
Contributor Author

Review points addressed in edca066 (only ANALYTICS.md changed; assistant.js is as before):

  • Weekly report: now says probes are counted and only a short sample is shown.
  • Policy: the owned task is triaging each weekly "Docs assistant: N unanswered questions" issue (owner @celo-org/devrel), closing it once its real gaps are doc issues. The monthly Redis read is gone; the Redis list stays as the raw store.
  • Raw-question rule: scoped to manual copies. The doc now states that the bot report currently pastes raw questions into a public issue, and that redacting it, skipping refused: true entries and expiring the log is tracked in celo-org/docs-ai-assistant#3.
  • Header source: names the /widget.js headers rule in docs-ai-assistant's next.config.ts (checked that it sets Access-Control-Allow-Origin: *).
  • gtag: the typeof window.gtag clause is dropped; the pass criterion is one assistant_question event in GA4 Realtime. task: make the assistant's existing telemetry reportable (GA4 custom dimensions, gtag after GTM swap, unanswered-question review) #2307 acceptance criterion 2 and its "monthly pull" line were updated to match.

mint broken-links passes.

@GigaHierz
GigaHierz requested a review from palango October 2, 2026 09:48

@palango palango left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this covers everything from my last review. The probe wording matches what route.ts does now, the raw-question rule is scoped to manual copies and points at docs-ai-assistant#3, and making triage of the weekly issue the owned task is simpler than a monthly Redis read. I checked the /widget.js rule in next.config.ts and the live header, and the post-swap check no longer leans on window.gtag.

Two optional nits inline. One more, outside this PR: the Gap 2 paragraph in #2307 still says GTM's Google tag should define window.gtag, which contradicts what ANALYTICS.md now says. Worth fixing so the issue and the doc agree.

Comment thread ANALYTICS.md
- **Weekly:** an automated report files a "Docs assistant: N unanswered questions" issue listing questions that returned no citation, with injection and enumeration probes shown as a count and only a short sample.
- **Owned task:** `@celo-org/devrel` triages each weekly issue and closes it once its real gaps have been turned into doc issues. Questions that were refused as off-topic carry `refused: true` and are not gaps.
- The Redis list `docs-assistant:questions` is the raw store for digging deeper. It holds raw question text.
- Do not copy question text into other issues, PRs or commits; describe the topic instead. The weekly bot report currently pastes raw questions into a public issue. Redacting it, skipping `refused: true` entries and expiring the Redis log is tracked in the `docs-ai-assistant` repository, issue #3.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this page is public, and on docs.celo.org "issue #3" renders as plain text with no repo. Could you use the full URL (https://github.com/celo-org/docs-ai-assistant/issues/3)?

Comment thread ANALYTICS.md

### Script hardening

`assistant.js` loads `https://docs-assistant.celo.org/widget.js` with `crossOrigin = 'anonymous'`; the host sends `access-control-allow-origin: *` from the `/widget.js` headers rule in `docs-ai-assistant`'s `next.config.ts`, so the load works under CORS. Anyone editing that rule should know docs.celo.org depends on it. There is **no Subresource Integrity hash and no Content-Security-Policy** on this site: an `integrity` hash would break the widget on every widget deploy, and the Mintlify Starter plan offers no header configuration we have confirmed. Whoever controls `docs-assistant.celo.org` can therefore run script on every docs page, so changes to that host need the same review as a change to this repo (#2302).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the comment in docs-ai-assistant's next.config.ts says Mintlify exposes no header configuration on any plan, and that a <meta> CSP injected by our loader would apply to nothing. This line hedges to the Starter plan. Whichever is accurate, the two should agree.

@GigaHierz
GigaHierz merged commit 51a4967 into main Oct 2, 2026
5 checks passed
@GigaHierz
GigaHierz deleted the GigaHierz/2307-assistant-telemetry-review branch October 2, 2026 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants