Test RAG retry telemetry logging#82
Conversation
There was a problem hiding this comment.
Pull request overview
Adds retry telemetry and context-pack caching to the RAG answer generation path, and introduces regression tests to ensure retry reasons are recorded and context packing can be reused safely across retries.
Changes:
- Extend
RagAnswer.latencyTimingsto includecontext_pack_cache_hits,answer_retry_count, andanswer_retry_reasons. - Add a stable
packedContextCacheKey()and a per-answer in-memory cache to reuse packed context across retry attempts. - Add/extend tests to validate retry telemetry recording and packed-context cache key stability (including document-scope sensitivity).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tests/rag-context-budget.test.ts |
Adds coverage for packedContextCacheKey() stability and document-scope behavior. |
tests/rag-answer-fallback.test.ts |
Adds regression test for retry telemetry logging and asserts retry timing fields in an existing fallback scenario. |
src/lib/types.ts |
Extends RagAnswer.latencyTimings with new retry/cache telemetry fields. |
src/lib/rag.ts |
Implements stable cache keys + context-pack reuse across retries; records retry count/reasons into answer timings and query-log metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1515d239bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| context_pack_cache_hits: contextPackCacheHits, | ||
| answer_retry_count: answerRetryCount, | ||
| answer_retry_reasons: [...answerRetryReasons], |
There was a problem hiding this comment.
Preserve retry counters in fallback logs
When generation falls into this fallback path after a retry has already been scheduled (for example, the fast answer triggers fast_*_retry_strong and the strong model call throws), these counters are present on the returned fallbackAnswer.latencyTimings, but the logRagQuery metadata built in the catch block still only writes context_pack_latency_ms and omits context_pack_cache_hits, answer_retry_count, and answer_retry_reasons. That makes the production rag_queries telemetry lose the retry reason exactly for failed retry attempts, so the new retry logging is incomplete for generation-fallback cases.
Useful? React with 👍 / 👎.
…ved (#1230) Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…1233) PR #1217 squash-merged to main as 20300bb with PR required green and post-merge push CI green. Content verified on main: canRestoreFocusTo at both restore call sites, bounded reclaims, the event-driven settle window and no setInterval. #82 previously recorded racing the sync bot as the only workaround. Applying skip-branch-sync first stops the re-sync loop outright, which is what let #1217 hold a green head long enough to merge. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Opened a GitHub issue so the bot branch-sync CI stall is visible outside the ledger; point the #82 Source column at it. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Summary
Verification