fix(observability): stop MCP paying for the browser's comparison grid - #267
Merged
Merged
Conversation
…on grid service_performance answers a question about one service. It also returned the latency heatmap -- the twelve busiest services by every bucket in the window -- because the browser's performance page draws that grid and both callers shared one method. On the live demo that was about 85% of a 113 KB response describing twelve services the caller had not asked about, plus the second DuckDB query that built it. The grid is now opt-in through PerformanceOptions. The browser asks for it; the MCP tool does not, and skips the query rather than blanking the field afterwards, so the cost actually goes away. Also reattaches three doc comments that had been separated from what they document, and adds the guard that found the third. A declaration inserted between a doc comment and its subject is invisible to gofmt, go vet and the compiler: the new declaration inherits a comment describing something else and the original is left undocumented. errorRateAnomalySQL's doc had landed on minErrorRateStddev, ParquetStore.Trace's on TraceTotals, and attrsJSON's on attrBufPool -- where it had also gone stale, still promising a nil return from a function that returns "". Requiring every doc comment to begin with its own declaration's name would flag hundreds that simply do not follow that convention, and a guard that is mostly false positives gets muted; the first cut of this one flagged 17, nearly all of them legitimate. It checks the exact signature instead -- a doc comment whose first word names the declaration immediately below it -- which is what such an insertion produces and what nobody writes on purpose. One flag across 2,378 declarations, and it was real. The first version of that guard did not catch the bug it was written for: it walked only documented declarations, so it compared against the next documented one and stepped straight over the undocumented victim. It now walks every declaration in source order, verified by reintroducing the defect.
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
1 participant
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.
service_performanceanswers a question about one service. It alsoreturned the latency heatmap -- the twelve busiest services by every
bucket in the window -- because the browser's performance page draws that
grid and both callers shared one method. On the live demo that was ~85%
of a 113 KB response describing twelve services the caller had not asked
about, plus the second DuckDB query that built it.
The grid is now opt-in via
PerformanceOptions. The browser asks for it;the MCP tool does not, and skips the query rather than blanking the
field afterwards.
Also reattaches three doc comments separated from what they document, and
adds the guard that found the third. A declaration inserted between a doc
comment and its subject is invisible to gofmt, vet and the compiler.
errorRateAnomalySQL's doc had landed onminErrorRateStddev,ParquetStore.Trace's onTraceTotals, andattrsJSON's onattrBufPool-- where it had also gone stale, promising a nil returnfrom a function that returns
"".The guard checks the exact signature (a doc whose first word names the
declaration immediately below it), not the general convention: the first
cut flagged 17, nearly all legitimate. This one flags 1 across 2,378
declarations, and it was real. Its first version also failed to catch the
bug it was written for -- it walked only documented declarations and
stepped over the undocumented victim. Verified by reintroducing the
defect.