Skip to content

Commit 66a249b

Browse files
committed
merge fix/expand-estimator: guard the phantom-map estimator, harden the gate, scope the callees legend (verifier #2 #11)
2 parents ac0b352 + edf1c8f commit 66a249b

8 files changed

Lines changed: 106 additions & 16 deletions

File tree

.ripwire_quality_acks

Lines changed: 2 additions & 1 deletion
Large diffs are not rendered by default.

README.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,12 @@ tool refuses.
5353
[What it misses, and what to run next →](#what-it-misses-and-what-to-run-next)
5454

5555
**New (2026-08-15), each measured, gated, and re-derived on every CI run:**
56-
- **`--expand` answers "show me this function" in one call, −57.6% tokens** — an exact-name ask now
56+
- **`--expand` answers "show me this function" in one call, −47.3% tokens** — an exact-name ask now
5757
skips the ranked-map preamble by default and the body arrives with its file's sibling symbols and
5858
imports inline (`sibs=`/`inc=`), so the follow-up "what else is in this file?" call never happens.
59+
(Re-derived 2026-08-15 fix-expand round: `--expand=emitGrepReport` — 47,252 B classic 200-row-map
60+
bundle vs 24,911 B today's exact-name default, on this repo; the figure moves as the corpus grows
61+
and is not itself CI-gated, so re-measure before citing it in a future round.)
5962
- **`--grep` groups, deduplicates, and speaks boolean** — per-file grouping, identical-line collapse
6063
("this exact guard appears at 6 call sites" is the finding), and `--and=`/`--not=`/`--grep-scope=`:
6164
a two-term ask returns the *complete* answer at **−78%** of the single-term dump. Default view now
@@ -801,8 +804,8 @@ $ ripwire . --callers=rankGraphTeleport
801804
<s t="fn" n="runEval" p="./src/eval.h:168"/>
802805
<s t="fn" n="rankGraph" p="./src/graph.h:2057"/>
803806
<s t="fn" n="anchoredLexicalRank" p="./src/graph.h:2393"/>
804-
<s t="fn" n="churnRankedGraph" p="./src/main.cpp:11489"/>
805-
<s t="fn" n="runDefaultMap" p="./src/main.cpp:11604"/>
807+
<s t="fn" n="churnRankedGraph" p="./src/main.cpp:11527"/>
808+
<s t="fn" n="runDefaultMap" p="./src/main.cpp:11642"/>
806809
<s t="fn" n="getIndex" p="./src/mcpindex.h:946"/>
807810
</callers>
808811
```

src/graphlegend.h

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,25 @@ inline constexpr const char* kCallHierarchyLegendOpen =
127127
"lists the symbols that CALL of=; the callees form lists the symbols of= itself calls. of= is the selector "
128128
"you passed, defs= how many DEFINITIONS that name resolved to (the rows UNION every def's neighbours), and "
129129
"count= the number of DISTINCT neighbour symbols (a floor, per counts_floor=), which the rows window with limit= and offset=. "
130-
"When emitted by callees, bodyless_defs= (when present) counts how many of the defs= are declarations with no body (header-only or forward-declared); "
131-
"zero callees may mean no body to read callees from rather than truly no dependencies. "
132130
"A neighbour that is an indexed function-like #define is a macro row (t=\"macro\", role=\"macro\" on the XML row): "
133131
"the edge crosses a macro expansion, not a plain call — rows carry no role= otherwise. ";
134132

133+
// V1 fix (verifier finding 3, 2026-08-15): bodyless_defs= is a CALLEES-only attribute — main.cpp's emitter
134+
// gates it behind `!wantCallers`, so a --callers document can never carry it. It used to sit inside
135+
// kCallHierarchyLegendOpen above, which both forms print, so every --callers call paid ~235 B for a clause
136+
// it could never need. Appended only on the callees form (see the call site in main.cpp), so the clause
137+
// still appears verbatim wherever the attribute CAN appear — legendcoveragecheck's callees-side coverage is
138+
// unaffected; only the callers-side dead weight is gone.
139+
inline constexpr const char* kCallHierarchyLegendCalleesOnly =
140+
"When emitted by callees, bodyless_defs= (when present) counts how many of the defs= are declarations with no body (header-only or forward-declared); "
141+
"zero callees may mean no body to read callees from rather than truly no dependencies. ";
142+
143+
// The composed opener, one call for the caller — keeps the wantCallers/callees branch out of
144+
// runCallHierarchy (already this file's largest dispatcher) rather than adding a ternary at the call site.
145+
inline std::string callHierarchyLegendOpen( bool wantCallers )
146+
{
147+
return wantCallers ? std::string( kCallHierarchyLegendOpen )
148+
: std::string( kCallHierarchyLegendOpen ) + kCallHierarchyLegendCalleesOnly;
149+
}
150+
135151
} // namespace rw

src/main.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5849,12 +5849,17 @@ std::optional<int> runCallHierarchy( const MainDispatch& d )
58495849

58505850
// §H4 §3.4: the FIRST legend these two verbs have ever shipped (0 bytes before — which is why every
58515851
// one of their root attributes sits in test/legendcoverage_baseline.txt), and the floor marker that
5852-
// is the round's honest half. ONE string for both forms, printed BEFORE the format branches so the
5852+
// is the round's honest half. ONE opener for both forms, printed BEFORE the format branches so the
58535853
// columnar and default shapes carry the identical disclosure. JSON has no comment-node analogue, so
58545854
// there the marker travels as the counts_floor key on the root object instead.
5855+
// V1 fix (verifier finding 3): bodyless_defs= is callees-only (main.cpp gates the attribute itself
5856+
// behind !wantCallers a few lines up), so its defining sentence rides along only on the callees
5857+
// form — a --callers call no longer pays for vocabulary it can never emit. The wantCallers branch
5858+
// lives in rw::callHierarchyLegendOpen (graphlegend.h), not here, so it does not add to this
5859+
// already-large dispatcher's own complexity.
58555860
if( !cfg.json )
58565861
{
5857-
std::printf( "%s%s-->", rw::kCallHierarchyLegendOpen, rw::graphCountDisclosure().c_str() );
5862+
std::printf( "%s%s-->", rw::callHierarchyLegendOpen( wantCallers ).c_str(), rw::graphCountDisclosure().c_str() );
58585863
}
58595864

58605865
// --format=columnar (RESEARCH lever 1): the same page window, re-encoded as a path-table + parallel
@@ -12220,7 +12225,14 @@ int runDefaultMap( const MainDispatch& d )
1222012225
// + the pre-rendered <bodies> + "</ctx>". Rendering-and-measuring beats arithmetic here: the map's
1222112226
// est_tokens digits depend on the payload charge, and a probe that prices a shape it then fails to
1222212227
// build is the exact climbCeilingLadder failure mode this file already documents.
12223-
const std::size_t bundleBytes = ( sizeof( "<ctx>" ) - 1 ) + measureEmittedMapBytes( mapTopK, payloadTokens )
12228+
// V1 fix (verifier finding 1, 2026-08-15): mapTopK==0 means UNLIMITED inside serialize(), not "no
12229+
// map" — an unguarded call here priced the whole-repo map (~1MB on this tree) that the emission
12230+
// path below never prints (mapTopK==0 skips straight to the topK>0 branch's `else`, see the two
12231+
// guarded siblings at the ceiling verdict and the topK>0 emission gate). Same guard here: a map
12232+
// that will not be emitted must not be charged, exactly like every other measureEmittedMapBytes
12233+
// call site in this function.
12234+
const std::size_t bundleBytes = ( sizeof( "<ctx>" ) - 1 )
12235+
+ ( mapTopK > 0 ? measureEmittedMapBytes( mapTopK, payloadTokens ) : 0 )
1222412236
+ bodiesSection.xml.size() + ( sizeof( "</ctx>" ) - 1 );
1222512237
wholeFile = rw::renderWholeFiles( ing, expandNodes, redactPtr, d.notesPtr, cfg.compress ); // D2: shaped candidate
1222612238
ExpandServeChoice choice = chooseExpandServe( bundleBytes, wholeFile, cfg.packBudgetBytes );

test/expandmodecheck.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ cd "$TMP"
4747
fileBytes="$( wc -c <"$TMP/fix/small.c" | tr -d ' ' )"
4848

4949
# ── (1) small-file symbol: the whole file is the cheapest complete answer ─────────────────────────────
50+
# V1 fix (verifier finding 1, 2026-08-15): smallProbe is an EXACT-NAME match, so its own default bundle
51+
# is now the LEAN (topk_default="0") body — not map+body — per the same V1 default arm (2) already
52+
# accounts for on bigProbe007. That makes small.c's fixture genuinely byte-minimal a REQUIREMENT, not
53+
# cosmetic: with the estimator's phantom-map bug fixed, a lean body beats any file bytes it does not
54+
# have to pay a descriptive header comment for, so small.c carries NO leading comment (unlike big.c,
55+
# whose bulk swamps a header either way) — a comment here was previously masking the correct comparison
56+
# by inflating the file side just enough to keep mode="whole-file" for the wrong reason.
5057
"$BIN" fix --expand=smallProbe --no-cache >"$TMP/small.xml" 2>/dev/null
5158
grep -q 'mode="whole-file"' "$TMP/small.xml" \
5259
&& ok "(1) small-file --expand serves mode=\"whole-file\"" \

test/expandmodefix/small.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
// expandmodefix/small.c — the SMALL-file case for test/expandmodecheck.sh: the whole file is byte-cheaper
2-
// than the default --expand bundle (map + body), so auto mode-selection must serve mode="whole-file".
31
int smallProbe( int value )
42
{
53
return value * 2 + 1;

test/expandtopk0check.sh

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,56 @@ else
112112
ok "(E) bundle mode ships no ranked map either"
113113
fi
114114

115+
# ── (G) V1 fix regression guard (verifier finding 1, 2026-08-15) — the shape (B) and (E) both
116+
# structurally could not exercise. (B) forces bundle mode via --pack-budget-bytes=10, so the
117+
# estimator's own value never gets compared against a real whole-file candidate. (E)'s
118+
# bigProbe007 fixture is tiny and hand-built, so even a wildly wrong bundle estimate happens not
119+
# to flip the mode there. Neither arm runs at the DEFAULT budget on a REAL, non-trivial repo — the
120+
# one place the defect actually manifested: measureEmittedMapBytes(mapTopK, …) was called
121+
# UNGUARDED for the bundle-size estimate, and mapTopK==0 means UNLIMITED inside serialize() (not
122+
# "no map"), so the estimator priced a ~1 MB whole-repo map that the emitter never intended to
123+
# print. chooseExpandServe then compared that phantom bundle against the real whole-file candidate
124+
# and picked mode="whole-file" — serving the ENTIRE 48 KB src/darkflags.h in place of the ~1 KB
125+
# body the exact-name default promises. RED on the pre-fix binary
126+
# (reason="file 48222B &lt; bundle 1054283B", mode="whole-file"); GREEN once the estimator is
127+
# guarded exactly like its two siblings at the ceiling verdict and the topK>0 emission gate
128+
# (`mapTopK > 0 ? measureEmittedMapBytes(...) : 0`).
129+
"$BIN" "$ROOT" --expand=endsWithView --no-cache >"$TMP/real_default.xml" 2>"$TMP/real_default.err"
130+
"$BIN" "$ROOT" --expand=endsWithView --no-cache --top-k=0 >"$TMP/real_tk0.xml" 2>/dev/null
131+
realTk0Bytes=$( wc -c < "$TMP/real_tk0.xml" | tr -d ' ' )
132+
133+
if grep -q 'mode="whole-file"' "$TMP/real_default.xml"; then
134+
no "(G) default --expand=endsWithView on the real repo wrongly served whole-file: $( grep -oE '<ctx[^>]*>' "$TMP/real_default.xml" )"
135+
else
136+
ok "(G) default --expand=endsWithView on the real repo correctly stays in bundle mode"
137+
fi
138+
139+
# (G-a) the SERVED BODY (everything but the <ctx ...> opening tag's own mode=/reason= decoration, which
140+
# composing with M6 is the documented, (E)-gated contract) must be byte-identical to explicit
141+
# --top-k=0's — proving the estimator and the emitter now agree on what mapTopK==0 means.
142+
sed 's/<ctx[^>]*>//' "$TMP/real_default.xml" > "$TMP/real_default_body.xml"
143+
sed 's/<ctx[^>]*>//' "$TMP/real_tk0.xml" > "$TMP/real_tk0_body.xml"
144+
diff -q "$TMP/real_default_body.xml" "$TMP/real_tk0_body.xml" >/dev/null \
145+
&& ok "(G-a) default's served body is byte-identical to explicit --top-k=0's" \
146+
|| no "(G-a) default's served body diverges from explicit --top-k=0's — the estimator or the emitter disagree on what mapTopK==0 means"
147+
148+
# (G-b) when topk_default="0" is in effect and a reason= fires, the bundle byte count it PRICES must be
149+
# the REAL served size (== the --top-k=0 byte count), never a phantom map-inclusive estimate. This
150+
# is the precise, load-bearing number the V1 defect corrupted.
151+
pricedBundle=$( grep -oE 'reason="bundle [0-9]+B' "$TMP/real_default.xml" | grep -oE '[0-9]+' )
152+
if [ -n "$pricedBundle" ]; then
153+
if [ "$pricedBundle" = "$realTk0Bytes" ]; then
154+
ok "(G-b) reason= prices the bundle at exactly the --top-k=0 byte count (${pricedBundle}B) — no phantom map"
155+
else
156+
no "(G-b) reason= priced the bundle at ${pricedBundle}B but --top-k=0 actually serves ${realTk0Bytes}B — the reason= string still prices a map that mapTopK==0 will never emit"
157+
fi
158+
else
159+
no "(G-b) no reason=\"bundle NNNB ...\" clause found on the real-repo default root — unexpected mode, see (G) above: $( grep -oE '<ctx[^>]*>' "$TMP/real_default.xml" )"
160+
fi
161+
115162
# ── (F) well-formedness + determinism ─────────────────────────────────────────────────────────────────
116163
if command -v xmllint >/dev/null 2>&1; then
117-
for f in uniq dup5 dup tk5 tk0 big; do
164+
for f in uniq dup5 dup tk5 tk0 big real_default real_tk0; do
118165
xmllint --noout "$TMP/$f.xml" 2>/dev/null && ok "(F) $f.xml well-formed" || no "(F) $f.xml fails xmllint"
119166
done
120167
else

test/usesselectorcheck.sh

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,16 @@ grep -q 'srcmut_sigchange' "$TMP/err" && no "the constant nonsense suggestion (s
107107
# ── (d) plain name and canonical-id forms stay byte-identical to pre-fix (only file:name changed) ───────
108108
# NoteIndex::empty's canonical id ("path::scope::name") — a scoped method, so it actually carries "::"
109109
# (a scope-less free function's canonical id degrades to its bare name and can't test this branch).
110-
# The id is looked up live via --expand (it embeds the corpus path exactly as invoked — "." vs an
111-
# absolute ROOT produce different id= strings — so it must never be hardcoded).
112-
CANON_ID="$( "$BIN" "$ROOT" --expand='src/notes.h:empty' --no-cache 2>/dev/null | grep -o 'id="[^"]*NoteIndex::empty"' | head -1 | sed 's/^id="//;s/"$//' )"
113-
[ -n "$CANON_ID" ] || { no "could not look up NoteIndex::empty's canonical id via --expand"; CANON_ID="./src/notes.h::NoteIndex::empty"; }
110+
# The id is looked up live via --outline, not --expand (it embeds the corpus path exactly as invoked —
111+
# "." vs an absolute ROOT produce different id= strings — so it must never be hardcoded). V1 fix
112+
# (verifier finding 1, 2026-08-15): --expand's own exact-name default now correctly picks whichever of
113+
# lean-bundle/whole-file is genuinely cheaper (the bug this fix closed made whole-file win here for the
114+
# WRONG reason — a phantom map size — which is what let a bare --expand double as an id= lookup before).
115+
# Bundle mode's <b> body tag carries no id= at all, so --expand is no longer a mode-independent way to
116+
# fetch a canonical id; --outline always rides the classic 200-row map (no V1 lean default applies to
117+
# it) and its <s> rows carry id= unconditionally, so it is the stable lookup path here.
118+
CANON_ID="$( "$BIN" "$ROOT" --outline='src/notes.h:empty' --no-cache 2>/dev/null | grep -o 'id="[^"]*NoteIndex::empty"' | head -1 | sed 's/^id="//;s/"$//' )"
119+
[ -n "$CANON_ID" ] || { no "could not look up NoteIndex::empty's canonical id via --outline"; CANON_ID="./src/notes.h::NoteIndex::empty"; }
114120
BARE_A="$( uses_elem 'buildGraph' )"
115121
BARE_B="$( uses_elem 'buildGraph' )"
116122
[ "$BARE_A" = "$BARE_B" ] && ok "bare-name form is stable/reproducible: $BARE_A" || no "bare-name form not reproducible"

0 commit comments

Comments
 (0)