fix(mcp): match latency report by path, not length delta (closes #3642) - #3643
Merged
Merged
Conversation
…ntza#3642) analyzeFile snapshotted getLatencyReports().length before the dispatch and sliced the post-dispatch array from that offset to find the report the dispatch had just appended. dispatcher.ts pushes onto a 100-entry ring and shifts once it is over capacity, so a full ring's length stays at 100 across the push. Once 100 dispatches had run, the length delta was always 0 and the slice always empty, so analyzeFile silently returned undefined for `latency`, `fileKind`, and `lsp` on every analysis after that point. Match against the full current ring by resolved path instead, scanning from the tail so the newest report for a path wins over a stale one further back in the ring.
Contributor
|
Thanks for your first pull request! A maintainer will review it soon. Please make sure CI passes and the PR template is filled out. |
…efs apmantza#3643) The ring stores report objects and getLatencyReports() returns a shallow copy, so a reference delta survives the push+shift at capacity where a length delta cannot. Take the single appended reference; with concurrent appends keep only pathsEqual matches and never fall back to another file's report.
…efs apmantza#3643) The exact-pin glossary sweep pins the `filter` identifier per file; the two .filter(...) calls raised clients/mcp/analyze.ts from 1 to 3 against a pin of 1, whose remediation is to route around the spelling rather than raise the pin. Both loops are equivalent and keep newest-match-wins. The test also spread a `never`-typed literal (TS2698), so the cast now sits on the array the mock consumes.
) The dispatcher already knows which report it appended, so return it instead of letting the caller infer it from the ring. That deletes the before/after reference scan in analyze.ts and closes the residual same-path concurrency ambiguity: each caller receives its own report. The glossary pin for clients/mcp/analyze.ts shrinks 6 -> 5 with the removed branch.
|
Owner
|
thanks @drakeo338. merged. |
This was referenced Sep 29, 2026
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
3 participants
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.
Why
analyzeFilesilently dropslatencyonce the dispatcher's 100-entry ring is full.Notes for the reviewer
Change outline
Summary
Closes #3642.
analyzeFilematched its dispatch's report by slicinggetLatencyReports()from a before-dispatch length. The ring pairs every push past capacity with a shift, so a full ring's length never changes and the slice was always empty, silently droppinglatency. Now it scans the ring tail-first by resolved path.Type of change
Area
Checklist
npm run build; the full suite is CI's job.npm run mutation:diffwasn't executed); the red/green pair below covers the same guard.npm run lintpassesnpm run build:distsucceeds if I changed code underclients/,commands/,tools/, orindex.tspackage-lock.jsonis in sync withpackage.json(regenerate with the exact npm pin inpackage.json'spackageManagerfield) — not applicable, no dependency changedAGENTS.mdis updated if this PR changes behavior, commands, conventions, or invariants documented there — not applicable.changelog/<branch-or-slug>-<short-desc>.mdhas one valid entry in this PR for any user-facing change (Added/Changed/Deprecated/Removed/Fixed/Security) — see .changelog/README.md; internal-only test/refactor PRs may skip it(closes #NNN)or(refs #NNN)Tests
Test assessment
This file pins
analyzeFile's per-runner result shape; nothing is made redundant.Blast radius
Callers:
lens-engine.ts,runtime-tool-call.ts,complexity-client.ts,generated-artifacts.ts,mcp/review.ts. Only which report object is selected changes; same bounded-array scan cost.Observability
No new failure path; no record added.
Class sweep
Length-delta matching against a shifting ring; the only such consumer among the tree's other capacity-bounded rings (
degradation-ledger.ts,partial-edit-apply.ts,lsp/client.ts'sstderrRing,runtime-coordinator.ts's_mutationReceipts):Class of size 1.