fix(telemetry): tracer recognizes file:* spans for oddkit_get fast path#139
Merged
fix(telemetry): tracer recognizes file:* spans for oddkit_get fast path#139
Conversation
Follow-up to #138. The streaming-race fix (#138) corrected the timing bug, but oddkit_get for klappy:// URIs still records cache_tier="none" because of a *separate* defect: the tracer's _indexSource setter only recognized 'index' and 'index-build' labels. runGet for klappy:// URIs takes a fast path that skips getIndex entirely (URI → path is computable without the index). It calls getFile directly. The fetcher emits 'file:${path}' spans (memory/r2/build), but the setter's exact-match check (label === 'index' || label === 'index-build') ignored them. Result: _indexSource stayed null, blob9 = 'none'. This is the dominant get path: 'klappy://' URIs make up ~95% of oddkit_get calls. So even after #138, ~95% of get calls had cache_tier='none' — defeating the dashboard's purpose for that tool. Fix: extend the setter to also recognize 'file:*' labels alongside 'index' / 'index-build'. First-wins guard preserved, so: - runSearch: 'index' span fires first → index tier wins. Subsequent 'file:*' spans for hit fetches are ignored. blob9 = index tier (correct, that's the primary work). - runGet (klappy://): only 'file:*' span fires → file tier wins. blob9 = file tier (correct, that's the primary work). - runGet (kb://, odd://): 'index' span fires first (URI lookup), then 'file:*'. Index wins. blob9 = index tier (acceptable approximation for the ~5% of non-klappy gets). - 'file-r2:*' (R2 miss with source='miss') excluded by adding 'source !== "miss"' guard. 'miss' is not a tier and must not appear in blob9. Internal field name (_indexSource) and getter name (indexSource) stay unchanged. The public envelope key 'index_source' in tracer.toJSON() is part of the response contract and renaming would be a breaking change. The doc-comment on the getter is updated to reflect the broader semantic (primary cache tier, not strictly the navigability index). Regression tests added (4): 1. file:* span with each tier (memory/r2/build) populates indexSource 2. Index wins when fired before file spans (search pattern) 3. file-r2:* miss spans correctly excluded 4. Original index-only behavior preserved (no regression) Test count: 18 → 22 passing.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
oddkit | 904d804 | Commit Preview URL Branch Preview URL |
Apr 26 2026, 02:57 AM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #138. The streaming-race fix corrected the timing bug, but
oddkit_getforklappy://URIs still recordscache_tier="none"because of a separate defect: the tracer's_indexSourcesetter only recognized"index"and"index-build"labels.The remaining gap
runGetforklappy://URIs takes a fast path that skipsgetIndexentirely — the URI-to-path mapping is computable without the index. It callsgetFiledirectly. The fetcher emitsfile:${path}spans, but the setter's exact-match check ignored them. Result:_indexSourcestayed null,blob9 = "none".This is the dominant get path:
klappy://URIs make up ~95% ofoddkit_getcalls. So even after #138, ~95% of get calls hadcache_tier="none"— defeating the dashboard's purpose for that tool.Fix
Extend the setter to also recognize
file:*labels alongsideindex/index-build. First-wins guard preserved:runSearchindex→file:*(per hit)runGetklappy://file:*onlyrunGetkb:///odd://index→file:*runVersion/runTime"none"(correct — no fetch)file-r2:*(R2 miss withsource="miss") is excluded by an explicitsource !== "miss"guard."miss"is not a tier and must never appear inblob9.Naming honesty
The internal field name (
_indexSource) and getter name (indexSource) stay unchanged. The public envelope keyindex_sourceintracer.toJSON()is part of the response contract — renaming would be a breaking change for any consumer reading the trace. The doc-comment on the getter is updated to reflect the broader semantic: "primary cache tier" rather than strictly the navigability index.Regression tests added (4)
file:*span with each tier (memory/r2/build) populatesindexSourcefile-r2:*miss spans correctly excludedTest count: 18 → 22 passing.
What this completes
After #138 + this PR, every tool with a real data fetch records its actual tier:
oddkit_search,oddkit_orient,oddkit_catalog,oddkit_challenge,oddkit_encode,oddkit_preflight,oddkit_validate,oddkit_gate→ index tier (the dominant fetch they all perform)oddkit_get(klappy://) → file tier (the actual document fetch)oddkit_get(kb://, odd://) → index tier (URI lookup is the index hit, ~5% of gets)oddkit_version,oddkit_time→"none"(correctly — no fetch)cache_tierbecomes a meaningful field for every tool in the dashboard.Note
Low Risk
Low risk: changes are limited to how
RequestTracerderives the single telemetrycache_tiervalue and adds tests to lock behavior; it doesn’t affect request handling beyond reporting.Overview
Fixes telemetry
cache_tierattribution foroddkit_getfast-path requests by lettingRequestTracertreatfile:*spans (in addition toindex/index-build) as the primary tier, while keeping a first-wins rule and ignoringsource="miss".Adds integration tests covering file-only paths, index-before-file precedence, exclusion of
file-r2:*miss spans, and a regression check for the original index-only behavior.Reviewed by Cursor Bugbot for commit 904d804. Bugbot is set up for automated code reviews on this repo. Configure here.