Cap a name-collision edge, the fabrication provenance cannot see - #74
Open
inth3shadows wants to merge 2 commits into
Open
Cap a name-collision edge, the fabrication provenance cannot see#74inth3shadows wants to merge 2 commits into
inth3shadows wants to merge 2 commits into
Conversation
…annot see `provenance` marks an edge codegraph SYNTHESIZED. It says nothing about an edge codegraph extracted and then resolved WRONG, which arrives with provenance = NULL and confidence = 0.9 — our top trust tier. Reproduced against the installed 1.5.0 bundle (codegraph #66): a project with a module-level `append`, and `rows["k"].append(2)` matched onto it as {"confidence":0.9,"resolvedBy":"exact-match","refName":"append"} an edge that does not exist, arriving as one we fully trust. A wrong edge here corrupts the one thing testgraph answers. `metadata.resolvedBy` is the discriminator and was already in the index, unread. `import` / `qualified-name` rest on evidence in the source; `exact-match` rests on a bare name. Capped to 0.5 — below LOW_CONFIDENCE, so it flags — but ONLY when the target's name is shared by more than one symbol. On the codegraph repo's index, 16,569 of 29,440 calls edges are exact-match, so capping all of them would flag 56% of the graph and make VERIFY MANUALLY meaningless; the ambiguity filter narrows it to 8,407, and an exact-match on a unique name had nothing to get wrong. An unrecognized or missing `resolvedBy` scores exactly as before: a codegraph change can cost us this protection, never invent a flag. `verify_manually` was a bare boolean; a flag nobody can act on is noise. It now carries the reason — synthesized edge, name collision, or a low confidence the index itself reported. Measured, because the worry was flooding the flag. 300 random single-symbol changes per repo, counting journeys that go trusted -> flagged: honeyslate 324 journey-selections 0 newly flagged (0.0%) signedintake 261 journey-selections 8 newly flagged (3.1%) testgraph 66 journey-selections 0 newly flagged (0.0%) `max over paths of (min over edges)` absorbs nearly all of it — one import-resolved route in leaves a journey untouched. Edge count was the wrong unit to reason about. 270 tests, OK.
…are `reason`
Four defects from the dual review of this PR.
**The reason was inferred from the number, and the number is not unique to a
cap.** `0.5` is a tier real indexes report directly — `LOW_CONFIDENCE`'s own
comment says so. On honeyslate all 37 reach-kind edges at 0.5 target a name that
is unique in the index, so every "name-collision" label was false: 11 of 11 at
journey level, and it reproduced on this repo's own fixture, where `mid_a`
carries a plain {"confidence":0.5} and no `resolvedBy` at all. The test meant to
defend this only compared the two cap constants to each other, which cannot see
a collision with an index-reported value.
`impacted_closure(..., with_reasons=True)` now carries a cap token through the
CTE beside `conf`; each hop keeps the token of whichever cap produced the value
that survived its `min`, a tie going to the edge as the more specific fact.
Termination is unaffected — the token comes from a four-element set, so the
(id, conf, cap) triple space is still finite. Measured after: 33 flagged entries
on honeyslate, all correctly `metadata`, and zero name-collision labels on a
target that is not ambiguous.
**The ambiguity set was rebuilt per call.** It is an index-wide GROUP BY (~80 ms
on a 17k-node index) and `export.build_map` runs a closure per node, so it was
quadratic — 2.4-2.7x on the map builds, projecting ~21 minutes of identical
redundant work on a large index. TEMP tables are per-connection, so the table's
presence is the cache key and needs no bookkeeping; `refresh_ambiguous_names` is
the escape hatch for a connection that outlives a re-index, and stale data there
can only misgrade a cap, never change closure membership. honeyslate build_map:
0.89s -> 0.59s against 0.53s on main.
**`reason` meant two things.** It already meant "why this row is listed at all"
on the bare degrade rows, and two tests read its ABSENCE as proof a journey was
genuinely selected by the closure. The cap explanation is now `weak_reason`, so
that discriminator stays true.
**The select half had no coverage.** Deleting the attachment used to leave the
suite green; it now fails.
276 tests, OK.
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.
provenancemarks an edge codegraph synthesized. It says nothing about an edge codegraph extracted and then resolved wrong, which arrives withprovenance = NULLandconfidence = 0.9— this repo's top trust tier. A wrong edge there silently corrupts the one thing testgraph answers.Reproduced against the installed codegraph 1.5.0 bundle (codegraph #66) — a project with a module-level
append, androws["k"].append(2)extracted as a bareappendand matched onto it:The fix
metadata.resolvedByis the discriminator, and it was already in the index — just unread.import/qualified-namerest on evidence in the source;exact-matchrests on a bare name.exact-matchonto a name more than one symbol has is now capped atAMBIGUOUS_NAME_MATCH_CONFIDENCE = 0.5, belowLOW_CONFIDENCE, so it flags.The ambiguity filter is load-bearing. On the codegraph repo's own index:
Capping all exact-match would flag 56% of the graph and make
VERIFY MANUALLYmean nothing. An exact-match on a unique name had nothing to get wrong, so it is left alone.An unrecognized or missing
resolvedByis scored exactly as before: a codegraph change can cost us this protection, but can never invent a flag.It does not flood the flag
That was the risk worth measuring — 4,106 capped edges sounded like a lot. Journey-level effect, 300 random single-symbol changes per repo, counting journeys that go from trusted to flagged:
max over paths of (min over edges)absorbs nearly all of it: one import-resolved route in leaves a journey untouched. The edge count was the wrong unit to reason about.Also
verify_manuallywas a bare boolean — a flag nobody can act on is noise. It now carries areason: synthesized edge, name collision, or a low confidence the index itself reported.Verification
python -m unittest discover -s tests— 270 tests, OKbuild_name_collision_fixturelives in its own in-memory db so the existing count-sensitive tests are untouched. It pins: the ambiguous exact-match is capped; an import-resolved edge to the same target is not; an exact-match on a unique name is not; a missingresolvedByis not; and the cap tiers stay numerically distinct (weak_edge_reasonreads the cap back off the value).What this does not do
testgraph now downgrades a fabricated edge; it does not stop one being written. The codegraph-side fix is separate (colbymchenry/codegraph#1704, plus the kernel mirror on our fork at
v1.6.0-em.2), and it does not reach these indexes until the binary that writes them is updated.