Skip to content

feat: appetizer latency metric + new LA click tracking#150

Closed
yodem wants to merge 3 commits into
waiting-sourcefrom
feature/sc-44755-44684-la-metrics-clicks
Closed

feat: appetizer latency metric + new LA click tracking#150
yodem wants to merge 3 commits into
waiting-sourcefrom
feature/sc-44755-44684-la-metrics-clicks

Conversation

@yodem

@yodem yodem commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Implements two Library Assistant stories, based on the appetizer/progress-trail work in PR #136 (waiting-source).

sc-44755 — time_to_appetizer Braintrust metric

Adds a numeric, aggregatable metrics.time_to_appetizer on the trace measuring user-perceived latency from request receipt to the moment the appetizer is served (seconds, one decimal). Only emitted when an appetizer is actually served (suppressed/error/no-result/stream-closed all skip it). The existing metadata.appetizer debug blob is kept.

Note on semantics: the metric stops at server-side enqueue (progress_queue.put), so it reads slightly lower than the PR #136 "appetizer visible 5.1s" (client paint). Happy to move it to SSE-flush if preferred — flagged on the story for discussion.

sc-44684 — new assistant_click feature_names

Reports four new click feature_names. Implemented via a dedicated data-feature-name attribute read by the host click tracker before the generic link/aria-label logic — chosen over the ticket's literal "aria-labels" because:

  • the context chip already has a real, dynamic aria-label (overwriting it would degrade screen-reader output);
  • the appetizer/thinking links are <a> elements that otherwise short-circuit to the generic "Response link" label.

Reporting is identical either way — GA4 keys on the feature_name value, not the source attribute.

Element feature_name
Context chip (reading ref) initial_location_link
Appetizer topic links related_topics_link
Thinking-steps toggle thinking_steps_toggle
Thinking-step ref links thinking_steps_text_link

Also drops stopPropagation on the appetizer link (kept preventDefault) so the click reaches the host tracker. Only document-level click listener is the menu outside-click-close, which is unaffected.

Notes

Test plan

  • Backend: deterministic integration test asserts metrics.time_to_appetizer is emitted (runs appetizer inline to avoid thread races). Full suite 45 passed.
  • Frontend build passes.
  • Cauldron validation outstanding — story sc-44684 requires validating the four feature_names fire in cauldron before deploy.

🤖 Generated with Claude Code

yodem and others added 3 commits June 10, 2026 16:51
Adds a "reading now" context chip at the top of the chat that displays
the Sefaria text ref currently open in the reader. Clicking it navigates
back to that ref via sefaria:bootstrap-url. Also removes version_language
from the get_text tool description to clean up thinking step labels.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sc-44755: add a numeric, aggregatable metrics.time_to_appetizer on the
trace measuring user-perceived latency from request receipt to the moment
the appetizer is served (seconds, one decimal). Only set when an appetizer
is actually served; the existing metadata.appetizer debug blob is kept.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sc-44684: report four new assistant_click feature_names. Add a dedicated
data-feature-name attribute (read by the host click tracker before the
generic link/aria-label logic) so anchors and the context chip report
clean labels without degrading accessibility:

- context chip      -> initial_location_link
- appetizer topics  -> related_topics_link (drop stopPropagation so the
                       click reaches the host tracker; keep preventDefault)
- thinking toggle   -> thinking_steps_toggle
- thinking ref link -> thinking_steps_text_link

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@gitvelocity-reviewer

Copy link
Copy Markdown

📊 Code Quality Score: 44/100

55 × 0.8 = 44

Category Score Factors
🔭 Scope 13/20 7 files across frontend (Svelte ×3), backend (Python ×2), i18n, tests; new context chip feature, metric promotion, UI restyling; cross-cutting analytics/i18n/observability
🏗️ Architecture 10/20 Global history.pushState monkey-patch (notable risk); data-feature-name analytics pattern is clean; metric promotion from metadata to numeric; no new services or major structural changes
⚙️ Implementation 12/20 URL parsing with multiple fallback paths; SPA navigation detection via pushState/replaceState patching; derived reactive state; CSS tooltip with pseudo-element transitions; reasonably complex but not algorithmically deep
⚠️ Risk 8/20 Global history.pushState mutation is permanent and affects all navigation; stopPropagation removal changes event flow; no database/auth changes; frontend changes are easily reversible
✅ Quality 10/15 Good integration test for time_to_appetizer metric with deterministic _SyncExecutor pattern; no frontend tests for context chip or history patching; some removed comments reduce documentation; i18n strings added
🔒 Perf / Security 2/5 Analytics is non-blocking; noopener/noreferrer on external links; no security concerns introduced; no benchmarks or threat model

Was this score accurate? 👍 Yes · 👎 No

Scored by GitVelocity · How are scores calculated?

@yodem

yodem commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by two focused per-story PRs off waiting-source: #151 (appetizer latency metric) and #152 (new LA click types, 3/4 — initial_location_link deferred until the context chip lands). This combined PR also carried unrelated context-chip code; the split removes it.

@yodem yodem closed this Jun 15, 2026
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.

1 participant