fix(review-telemetry): drop the unsound escape-distance derivation - #164
Merged
Conversation
The escape-distance query computed surfacer - injection - 1 over `gates[]` positions. That index space carries one position per gate however many times the gate ran, and the record nowhere interleaves one gate's invocations with another's, so the count it produced was not the number of gates that had the defect in front of them and missed it. Remove the query and the normalization text that specified the derivation, and state the prohibition in its place so the derivation is not reconstructed from the surviving fields. Replace the removed queries with two the record does support: fix-induced regressions per injecting gate, and fix-loop-sourced plan drift by injecting and surfacing gate.
The setting condition appeared both in the field's definition and in its normalization rule, and the two had already drifted: the normalization copy said "a fix loop earlier in this same run", with no relatum for "earlier". Read as "an earlier gate than the surfacer", a gate that re-surfaces a defect its own fix loop introduced would record `null`, which contradicts records the log already carries. Point the normalization rule at the definition instead of restating it. Drop the gates[] ordering rule. The escape-distance query was its only consumer, and no remaining query indexes the array; a repeated gate also makes no ordering of the aggregate entries equal the execution traversal. Keep the one-entry-per-gate constraint, which every per-gate aggregation depends on.
…ount Three one-liners filter on disposition "actionable" while their comments claim every defect of the kind. The field definition says any non-null injected_at_gate marks a fix-induced regression, and finding-triage has dispositions besides "actionable" that a real defect can carry, so the comments named a superset of what the pipelines emit. Narrow the comments rather than widening the filters: enumerating dispositions in a filter would drift silently against the finding-triage SSOT, and the record keeps every disposition either way.
2026.7.34 is taken by the review-root change already on main.
The escape-distance removal generalized one half of each of three parallel statements and left the other half in its old form, and the drift that produces was exercised inside this branch. - The schema-gating sentence generalized its injected_at_gate half to "every query reading that field" while its topic_opened_by half still named a specific query set, so a new query reading that field had no rule telling it to carry the guard. - The normalization rule for injected_at_gate became a pointer to the field's definition while duplicate_of_gate and topic_opened_by kept a second copy of theirs; editing one copy without the other is how the first copy drifted. - duplicate_of_gate's definition attached "earlier" to the gate rather than to the surfacing, which reads as excluding the gate that surfaced the defect itself on an earlier iteration. The next clause already rules that reading out, so the wording was the only thing at odds. Also drop the parenthetical calling `iterations` a re-run count. The same line collects "iterations run" and the record shape carries `"iterations": 1` for a gate that ran once, so the gloss was off by one on a field the log aggregates.
There was a problem hiding this comment.
Pull request overview
This PR updates the review-telemetry skill documentation to remove an unsound “escape-distance” derivation based on gates[] array positions, replacing it with supported aggregations and tightening the normalization rules around injected_at_gate. It also bumps the plugin marketplace version.
Changes:
- Removed the escape-distance derivation (and a related positional-assumption query) and added a normalization-rule prohibition against re-deriving that metric from the current record shape.
- Added two new jq aggregations that the record does support: actionable fix-induced regressions per injecting gate, and actionable fix-loop-sourced plan drift by injecting/surfacing gate.
- Updated schema-guard wording for
topic_opened_by/injected_at_gatereaders and bumped.claude-plugin/marketplace.jsonfrom2026.7.34to2026.7.35.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| skills/review-telemetry/SKILL.md | Removes the unsound escape-distance derivation, updates normalization rules, and adds supported actionable aggregations. |
| .claude-plugin/marketplace.json | Bumps marketplace version to 2026.7.35. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…able The cost-proxy sentence said `iterations` and the per-gate false-positive count are both reconstructable post-hoc, while the Do not fabricate rule three lines down names an iteration count lost to compaction as a case that must be recorded as `null` and reported in `gaps`. An executor that takes the first statement at face value has no reason to reach the second, which is the one that keeps an unrecoverable count out of the log. Drop the claim; the wall-clock prohibition it preceded states its own reason and did not rest on it.
This was referenced Jul 30, 2026
Closed
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
review-telemetry'sReading the logsection derived an escape-distance — for a surfaced defect, how many gates had it in front of them and still missed it — by subtracting two positions in thegates[]array.gates[]holds one entry per gate together with aniterationscount, so a gate that ran several times occupies one position, and the record carries no field ordering one gate's invocations against another's. A difference of two positions therefore counts neither invocations nor missed opportunities. It also went negative whenever a later gate's fix loop injected a defect that an earlier-positioned gate then surfaced.Remove the derivation rather than repair the formula, and state the prohibition in the normalization rule for
injected_at_gate, the field the derivation read.Closes #154.
Changes
All in
skills/review-telemetry/SKILL.mdunless noted.gates[]ordering rule (Array order = execution order), whose only consumer was the deleted query, and state the one-entry-per-gate constraint in its place. That constraint was implicit in the record shape and unwritten, and every per-gate aggregation depends on it.iterationsfrom one sentence: the parenthetical calling it a re-run count, off by one against a record shape that carries"iterations": 1for a gate that ran once, and the claim that it is reconstructable post-hoc, which the adjacent do-not-fabricate rule contradicts by naming an iteration count lost to compaction as anullcase.actionable, and their comments say so.duplicate_of_gate,topic_opened_by, andinjected_at_gateparallel treatment: each field's setting condition is stated once, inCollect the run's facts, with itsNormalization rulesentry pointing there instead of restating it.injected_at_gatewhile naming every reader fortopic_opened_by; both halves now say every reader of the field..claude-plugin/marketplace.json— version2026.7.34to2026.7.35.Impact
rg 'gates\[|injected_at_gate|topic_opened_by|duplicate_of_gate'over the repository matches nothing outsideskills/review-telemetry/SKILL.md, so no other file names any of the removed or redefined surfaces.injected_at_gate, which deleting both queries would otherwise have orphaned, is read by both replacements.skills/code-review-gate/SKILL.mdis the one skill that writes into a record; it namesconfig,gaps, and the skipped-gate omission rule, none of which this touches.No field leaves the write surface and the schema version stays at 3, so a record written before this change is still readable by every surviving query.
Test plan
The accumulated log is local and unpublished, so the run below is reported rather than linked. The shard it ran against holds one schema-3 record, so it exercises execution and output shape rather than coverage.
injected_at_gate. The second returns no rows: that record holds no finding whosetopicisplan-actual-drift, which is the condition its filter adds to the first query's.mdformatpre-commit hook, configured in.pre-commit-config.yaml, passes on the markdown this branch changes; a reader can re-run it from that config.Notes
#154 asked for the metric to be rebuilt on an invocation-level coordinate system, with three schema shapes to choose between. This PR does not do that. It removes the metric and forbids re-deriving it from the current record, so nothing in the file reports a number the record cannot support. Rebuilding it needs a coordinate system rather than a corrected formula, which is #159's subject; until that lands, the prohibition is what keeps the gap from being filled by another unsound derivation.
The prohibition carries no reference back to either issue on purpose, so that fixing the metric does not require finding and updating anything else in the file.
Two other comments in the same section overstate their queries in a different way — they describe a set their filter does not restrict, rather than reading a position the record does not carry. That is #163, filed against this repository.