fix(legend): compact answers printed map, impact, safe-delete and community counts with no definition; pins now fit per verb - #203
Open
joyful-ii-V-I wants to merge 10 commits into
Open
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s, --community and the map header printed attributes with no definition
Under --legend=compact (the MCP default) these answers carried attributes the compact legend never defined, because the
prose that defined them (if any did) is stripped and kCompactCompletenessTerms had no row:
- the map header: unresolved=, and the always-on files=/symbols=/edges=/shown=/ambiguous=/order= plus the conditional
roots=, changed=, skipped_oversize=, unindexed=/unindexed_exts=, escaped_root=, precise= (the full map legend defines
only shown=; serialize.h records why it cannot hold the rest);
- --impact: defs=, reaches=, radius_tested=, radius_untested=, importers=, shown_importers=, and <f lazy=> importer rows;
its columnar form's format=, <cols fields= n=> and lens= (lens= also on --order=stable's <r>);
- --callers, --callees, --impact and the MCP impact twin: tested="1" on <s> rows. No probe saw it, because test/fixture
holds no test; --impact=svector::push_back on this repo prints four;
- --safe-delete: radius_tested=, radius_untested=, t=, p=, defs=, ambiguous_callers=, dead_code_candidate=;
- --communities: drill=, shown_modules=, bridges=, isolated= and its isolated_doc/_decl/_header/_source split, modules=,
shown_bridges=, connected_singletons=, symbols=; --community=ID: dir=, label=, bridges=, partition=, modules=,
shown_bridges=; the <bridge> rows of both (a= b= from_label= to_label= edges=, to= to_label= edges=).
Definitions, each checked against its emitter (verbs_report.h, verbs_navigate.h runSafeDelete, serialize.h buildStats and
emitImportRowsXml, graph.h scanImporterEdges and isTestedByReach, columnar.h, quality.h). Root vocabulary every answer carries
reads in the purpose lines; everything that rides only some answers is a present-only term (element-qualified on <s>, <f>,
<bridge>, <cols> and <impact>, header-only for the map header's fields). Owner decision: per-verb pins that fit honest
definitions (docs/METHODOLOGY.md §9).
A design review read every new reading against its emitter before this commit and corrected three:
- --safe-delete t= p= read "its lowest-id def's kind and file:line". On a header-qualified file:name selector the decl->def
widening keeps the declarations (graph.h declToDefFollowThrough), so the lowest id can be one. Now "its lowest-id
match's", the unit defs= counts.
- the map header's changed= read "K git-changed files seed the PageRank teleport (0: uniform)". main.cpp counts INDEXED
files only, and a tree git cannot read counts 0. Now "K indexed git-changed files ... (0: uniform, incl. no git)".
- --communities' bridges= read "module pairs joined by a call edge" beside modules= "modules of 2+ symbols", but it counts
pairs that include one-symbol communities. Now "community pairs joined by a call edge".
It found the tested= reading missing ((D31), red first) and fourteen shortenings that keep each reading exact, all applied:
edges=, roots=, skipped_oversize=, unindexed=, escaped_root=, lens=, dead_code_candidate=, --communities' shown_modules=/
shown_bridges= (merged), symbols= and connected_singletons=, --community's bridges=, <bridge edges=>, <f lazy=>, and
unresolved= folded into the always-on files= clause. buildStats writes both fields into every header, and compactlegendcheck
(S) now accepts an unmarked always-on field spelled inside that clause; with unresolved= deleted from it, (S) FAILs on
unresolved= and on nothing else.
Pins (test/compactlegendcheck.sh). M held every verb under one 400 B (U) ceiling, (L) 4,100 B and a 400 B (M) ceiling. Rule:
the largest measured probe on the gate fixture, rounded up to the next 10 B, plus 10 B. pinFor is now a data table (schema,
pin, measured) with the rule stated once. schema old->measured->pin:
map 400->799->810; map-diff 400->789->800; pack-signatures 400->663->680; metrics 400->702->720; deps 400->245->260;
hotspots 400->264->280; clones 400->280->290; readability 400->224->240; nonlocal-state 400->344->360; ensemble
400->261->280; context-ratio 400->240->250; quality-panel 400->271->290; naming-calibration 400->206->220;
naming-consistency 400->261->280; comment-coherence 400->241->260; cochange 400->271->290; communities 400->807->820;
zoom 400->394->410; tree 400->238->250; seams 400->365->380; handoff 400->325->340; test-gate 400->400->410;
field-affinity 400->304->320; skipped 400->195->210; lint 400->240->250; lint-catalog 400->124->140; external-surface
400->171->190; scan-skills 400->147->160; owners 400->207->220; dead-code 400->297->310; quality-delta 400->212->230;
dmm 400->189->200; pr-context 400->399->410; stray-content 400->179->190; flags 400->159->170; doc-drift
400->205->220; notes 400->135->150; path 400->263->280; connect 400->392->410; impact 400->770->780; mentions
400->168->180; affected 400->339->350; verify 400->316->330; help-task 400->153->170; query 400->611->630; grep
400->345->360; match 400->260->270; lego 400->275->290; exemplar 400->232->250; around 400->707->720; callers
400->317->330; callees 400->369->380; uses 400->271->290; batch 400->142->160; safe-delete 400->708->720; at
400->161->180; from-trace 400->290->300; plan-lint 400->156->170; merge-scout 400->208->220; whereis 400->223->240;
community 400->719->730; layout 400->149->160; pack-task 400->327->340; pack-top-n 400->649->660; expand 400->265->280
(L) ten-verb loop: 4,100 -> 4,849 -> 4,900 (--impact 390 -> 770 B, --safe-delete 328 -> 708 B; nothing else moved).
(M) impact 400 -> 770 -> 780; uses 400 -> 271 -> 290; path_between 400 -> 263 -> 280; lego 400 -> 249 -> 260;
exemplar 400 -> 243 -> 260.
The compact saving, restated from measurement. --help promised "at least 50% of a small --callers/--uses/--impact/--affected
answer"; test/legendcostcheck.sh measures lookupLang and src/wrap.h in root-normalised bytes: callers 65.91% (M: 68.17%),
uses 63.79%, impact 46.17% (M: 52.78%), affected 64.73%. The minimum rounded down to a multiple of 5 is 45, so --help now says
"at least 45%" and the gate re-derives it from that sentence. callers and impact fell because lookupLang's rows carry
tested="1" and now pay its reading.
Published text: src/cli.h's --legend entry no longer promises "ONE <=400 B legend" and states the measured per-call drop,
2.8-5.8 KB (--callers/--callees/--uses/--impact/--edit-check=lookupLang, root-normalised: 2,769 B on --callers to 5,792 B on
--edit-check; it read 2.9-5.2 KB), with --edit-check 6.3 KB -> 0.5 KB (it read 5.2 KB -> <0.4 KB, already stale); src/main.cpp's
two comments and src/compactlegend.h's size comments stop quoting a 400 B ceiling; test/showcase_capture.py's demo note says
one compact comment. docs/COMMANDS.md regenerated (the one --legend line). test/printf_parity.manifest re-pinned: help_all
moved, 41 labels unchanged. Left as written: docs/EVALS.md's and bench/tokenaudit/README.md's quotes of "at least
50%" (dated 2026-09-09 round records), docs/EVALS.md's other dated lines, compactlegend.h's dated A6 hub_floor note,
test/mcpmanifestcheck.sh's comment quoting 2.9-5.2 KB.
Red -> green. The (D24)..(D29) rows FAILed 36 specs on the build before the sweep's last pass; (D30), the present-only mirror,
is green on both by construction and its controls assert each lack first. (D31)'s three rows FAILed on the last-pass build,
plain and ASan alike, and nothing else did; (D32), its mirror, is green by construction and fires with the reading spliced
into a legend whose document has no tested row. compactlegendcheck now: plain 125 PASS 0 FAIL, ASan 125 PASS 0
FAIL. legendcostcheck, docs/limits_build.py --check and docs/gatecount_build.py --check pass.
Byte identity against M's binary: 39 non-compact answers (the map, --impact and its columnar form, --safe-delete including
a header-qualified selector, --communities, --community, --zoom, --map-diff, --max-tokens, --order=stable, --callers/
--callees/--uses/--edit-check/--affected/--test-gate/--from-trace, test/chafix, --scip, a tree with a test, two roots,
refusals, MCP legend:"full", --help, --help=--uses) match in stdout, stderr and exit code; only --help=all differs. 26
compact answers keep rows and data comments identical (est_tokens aside, which prices the emitted bytes); only legend text
moves, on 20 of them.
quality-delta (against M) gates on two findings, both expected. One is short-horizon churn of kCompactLegendSpecs: four
sweeps and this review edited that table in one day, each adding or correcting readings. The other is the verbosity of
kCompactCompletenessTerms (47 -> 64 lines, bar 60): it holds one row per present-only reading, and splitting it would reorder
the clauses every existing compact legend prints. Minor: churn on kCompactCompletenessTerms, kCompactProsePrefixes (a
comment) and condArm.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test/printf_parity.manifest's help_all conflicted: both sides moved --help. Neither side's hash was the merged binary's, so main's was taken and the label re-pinned from the merged build; no other label moved. docs/COMMANDS.md regenerated from the same build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 9 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. This review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (9)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe compact legend adds definitions for more XML attributes and conditional fields. Validation now uses per-verb byte pins and expanded structural checks. Help text, documentation, comments, showcase text, and parity data reflect the revised measurements. ChangesCompact legend contract
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The compact legend, validation, help, and documentation changes are consistent and appear ready to merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 5 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Comment |
…printed with no definition Under --legend=compact two forms of the tested lens reached the reader undefined. <d tested="1"> signature rows (--pack-task --metrics; serialize.h, from computeQMetrics' tested[] column) matched no term, because the <s tested=1> reading is element-qualified on <s>. The columnar form of --callers/--callees/--impact names tested in <cols fields=> and writes a dense <tested> array of 1 and 0 (columnar.h emitColumnarTestedColumn), which no term read either; over a tree with no test that array is all zeros. Readings added to kCompactCompletenessTerms, each present-only: - <d tested=1>: a non-test row an indexed test transitively reaches (absent otherwise, never 0) - <tested> column: 1 = a non-test row an indexed test transitively reaches; 0 = none found, or a test row The column term keys on a <cols fields=> that lists tested rather than on a 1, so CompactCompletenessTerm gains valueItem and payloadHasAnyAttr reads one comma-separated item of the attribute's value (a defaulted parameter; its two callers are unchanged). test/compactlegendcheck.sh: (D33) the <d> reading on --pack-task=helper --metrics over a tree with a test; (D34) the column reading on columnar --callers/--callees/--impact, including an all-zero column over test/fixture; (D35) the mirror on five answers that lack each field. Red on e45bd3a, plain and ASan alike: the six (D33)/(D34) checks failed and nothing else did. Green on both after. No pin moved: every (U) probe's prose legend, the (M) legends and the (L) loop (4,849 B) measure the same before and after, because no pinned probe carries either field. Full-dialect answers are byte-identical against e45bd3a; compact payloads are identical and legends only gain the two readings (+105 B on a columnar callers/callees/impact answer, +95 B on --pack-task --metrics over a tree with a test). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s lane One conflict, test/printf_parity.manifest's help_all line: #139 and this lane both changed --help, so neither side's hash was right. Took main's side, rebuilt with --clean-first, and re-pinned from the merged binary (UPDATE_GOLDEN=1 printffmtparitycheck): help_all moved, the other 41 labels match main's manifest. docs/COMMANDS.md regenerated from the merged build equals the text merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s schema, its vocabulary undefined main.cpp's compactLegendHint derived one key from the flags in one order: map-diff, metrics, around, query, then the bundle verbs. A key names a spec only under its own root, and a key from the other family silently took that root's FIRST spec, so --pack-task --metrics, --from-trace --metrics and --pack-task beside --around all compacted as pack-signatures. No single flag order is right, because verb precedence interleaves the families: --pack-task and --from-trace answer over --around, while --around answers over --expand and --pack-signatures. The root now picks the family (compactMapLegendHint for <r>, compactBundleLegendHint otherwise), and the flags pick the key within it in the old order. Under the right schema the answer still printed attributes no compact reading named. The pack-task purpose line now spells the bundle's own vocabulary (task=, the <d>/<b>/<s>/<c>/<test> rows, of_top=, rel=, shared=, run=), checked against packtask.h. Present-only <d> terms read the lens facts serialize.h sigRowHead writes on a ranked bundle's rows (r=, cx=/ccx=, in=, and amp= under --metrics), and route= reads on <ctx>. --from-trace's <d> rows carry the same four facts and now read them too. Gate: compactlegendcheck (D36) reads the schema of four neighbouring combinations (the around-over-expand row is the one a flag reorder breaks), and requires every attribute name the compact --pack-task --metrics answer carries, on the CLI over two corpora and from MCP explore at its compact default, to be spelled NAME= in its legend. (D37) is the mirror. RED on 036c827: eleven (D36) checks FAILed and nothing else did. Green: compactlegendcheck ALL PASS on the plain build and on ASan. (D37) was shown able to fail by splicing each reading into an answer that lacks its field. Pins, by the lane's rule (measured, rounded up to 10 B, plus 10 B): pack-task 340 -> 820 (804 B), from-trace 300 -> 460 (445 B). A compact diff against 036c827 over 191 probes moved only pack-task and from-trace answers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n of zeros
--callers/--callees/--impact --format=columnar carry the tested lens as a dense <tested> column
(columnar.h emitColumnarTestedColumn): 1 where isTestedByReach holds, 0 on every other row, a test
row included. Their full legend printed kTestedRowLegend anyway, so test/fixture's --callers=distance
--format=columnar said tested is "never 0, omitted when it does not" beside <tested>0,0</tested>.
graphlegend.h gains kTestedColumnLegend ("1 = a non-test row an indexed test transitively reaches;
0 = none found, or a test row", the words of the compact column reading) and testedLensLegend( bool ),
the one form-to-sentence choice that callHierarchyLegendOpen and runImpact both call. The XML rows keep
the row sentence. The MCP impact twin has no columnar form and is unchanged.
Gate: impactpartitioncheck arm (5), beside arm (4), which already checks that the tested lens's legend
rides where the partition is read. Each columnar form's legend must read the 0s its column prints and
not carry the row sentence; each XML form must keep the row sentence, print no <s tested="0">, and
carry no column reading. RED on 036c827: the three columnar rows FAILed and nothing else did. Green:
ALL PASS.
Bytes: non-compact answers are byte-identical to 036c827's over 191 probes, except the six columnar
callers/callees/impact answers, which differ in this sentence alone. Compact answers are unaffected,
because the layer strips the sentence. graphlegendbudgetcheck measures the XML forms and did not move.
callHierarchyLegendOpen gains a third parameter; --edit-check finds one caller, 0 incompatible.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…il addresses — ripwirepubliccheck arm 5b flagged `…@d37.pt` The loop split each entry on '@' into a label and an output file, so strings like --pack-task=helper@d37.pt looked like addresses on the .pt TLD. The separator is now '|'; compactlegendcheck and ripwirepubliccheck both pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai review |
|
…re-check the --around pin Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… unproven_defs and same-scope readings grew the compact --around legend to 760 B Measured on the merged tree (main 69a9efa + this lane): --around=distance compact prose legend 760 B. The growth is definitions for attributes #210 added, so the pin moves to fit them; no other schema moved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Under
--legend=compact, several answers printed attributes their compact legend never defined:--impact's blast-radius counts;--safe-delete's verdict fields;--communitiesand--community's structure counts;tested="1"rows.A number an answer prints should be defined in that answer (
docs/METHODOLOGY.md§9). Defining them honestly did not fit the dialect's single 400 B per-answer pin, so the pins now follow the definitions.Terms added
files= symbols= edges= shown= ambiguous= unresolved= order=, plus, when present,roots= changed= skipped_oversize= unindexed= unindexed_exts= escaped_root= precise=.--impact:defs= reaches= radius_tested= radius_untested= importers= shown_importers=and<f lazy=>. The columnar form addsformat=,<cols fields=>andlens=.--safe-delete:radius_tested= radius_untested= t= p= defs= ambiguous_callers= dead_code_candidate=.--communities/--community:drill=,modules=,shown_modules=,bridges=,shown_bridges=,isolated=and its kinds,connected_singletons=,symbols=,dir=,label=,partition=, and the<bridge>rows.<s tested="1">rows on--callers,--callees,--impactand MCPimpact.Before this commit, a read-only review checked every definition against the code that emits it. It found three readings that were wrong, all corrected here:
--safe-deletet= p=now name the lowest-id match, not the lowest-id definition, because a header-qualified selector keeps its declarations.changed=counts only indexed git-changed files, and is 0 when git cannot be read.--communitiesbridges=counts community pairs, one-symbol communities included.Pins, per verb
Every compact schema was pinned at 400 B. Each is now pinned at its measured size rounded up to the next 10 B, plus 10 B.
PIN_TABLEintest/compactlegendcheck.sh.impactis pinned at 780 B.What
--helpnow says--legend=compactsaves "at least 45%" of a small--callers/--uses/--impact/--affectedanswer, down from 50%. The measured savings are--callers65.91%,--uses63.79%,--impact46.17% and--affected64.73%.legendcostcheckreads the number from--help.docs/COMMANDS.mdand thehelp_allpin intest/printf_parity.manifest. No other label moved.Verification
gates=34 pass=34 skip=0 fail=0, covering the compact legend, legend coverage, drift and cost, graph legend budget, MCP parity and manifest, printf parity, help budget, flag table, COMMANDS.md, showcase capture, README drift, est-charge, decl→def, the scip and sidecar gates, and every communities, impact, zoom and safe-delete gate.--help=allis the only difference.docs/limits_build.py --checkanddocs/gatecount_build.py --checkexit 0.kCompactCompletenessTermsgrowing from 47 to 64 lines against a bar of 60, plus short-horizon churn onkCompactLegendSpecs. Both are tables that gain one row per definition; the lane commit explains this.Left for a follow-up
tested="1"is still undefined under compact on--pack-task's<d>body rows and in the columnartestedcolumn.🤖 Generated with Claude Code