Skip to content

fix(uncertainty): withdraw the real-absence claim over untracked source - #987

Open
nhuphuoc215 wants to merge 3 commits into
tirth8205:stagingfrom
nhuphuoc215:fix/untracked-sources-defeat-real-absence
Open

nhuphuoc215 wants to merge 3 commits into
tirth8205:stagingfrom
nhuphuoc215:fix/untracked-sources-defeat-real-absence

Conversation

@nhuphuoc215

Copy link
Copy Markdown

_staleness proves the graph current two ways: build commit vs HEAD, and the target file's mtime vs the build timestamp. Neither can see a file git has never been told about — the commit still matches, and a file with no row in the graph has no mtime to compare.

An incremental update finds changes through git diff, so that file is absent from the index however fresh the build is. The result is that the strongest sentence this module produces lands on the case it is least true for:

agent writes new_consumer.py, doesn't `git add` it
→ references_to(Foo) → 0
→ "'Foo' is indexed and the graph is current, so this 0 is a real absence"

It then stops looking for the file it just wrote.

This adds untracked source as a third staleness signal, called from _settled() so it runs on both paths that would otherwise report the graph as current — including the early return taken when a graph has no last_updated, which is where my first attempt silently did nothing.

Two things keep it quiet. "Source" comes from the extensions already in the index rather than a fixed list, so a repo whose languages you don't parse never gets flagged; and --porcelain already honours .gitignore, so build output stays out. It is empty-result-only like every other marker here, so nothing that carries results changes. Cost is one git status on a path that already shells out for _live_git_head, ~10ms, never the hot path.

Six tests, all of which fail when the signal is removed. ruff and mypy clean, full suite 2991 passed, 9 skipped, 2 xpassed.

One thing I'd like your call on

Measured on a 1,489-file TypeScript monorepo, a single untracked consumer moved callers_of 4→3, importers_of 7→6 and references_to 12→11. All non-zero — so this marker never fires for any of them. The shape that actually bites is low, not empty, and low is worse because nobody thinks to doubt it.

Extending the note to non-empty results would cover that. It only appears while untracked source exists, which is not the steady state, so "non-empty responses are unchanged" would still hold for virtually every call — the tokens are spent only in the window where the answer is wrong. Happy to push it onto this branch if you want it.

Relation to integration/token-efficiency-hardening

Composes with _find_content_mismatches rather than overlapping it: that pass walks files already in the graph, so it catches the revert round-trip but not a file the index has never held.

An incremental update discovers changes through git, so a source file that
was never added is absent from the index however current the build is.
Neither staleness signal can see it: the build commit still matches HEAD,
and a file the graph holds no row for has no mtime to compare.

The result is that the strongest sentence this module produces is attached
to exactly the case it is least true for. An agent writes a consumer, does
not add it, asks who references the symbol, and is told the zero is a real
absence -- so it stops looking for the file it just wrote.

Adds untracked source as a third staleness signal, evaluated on both paths
that would otherwise report the graph as current, including the early
return taken when a graph carries no build timestamp. Source is calibrated
from the extensions already in the index, so a repo whose languages this
build does not parse never reports a file the index was never going to
hold; --porcelain honours .gitignore, so build output stays out.

Empty-result-only, like every other marker here, so no response that
carries results changes.
@github-actions

Copy link
Copy Markdown

code-review-graph review

Overall risk: 0.65 (MEDIUM) — 14 changed function(s)/class(es), 2 affected flow(s), 5 test gap(s)

Risk-scored changes

Risk Level Symbol Location Tested
0.65 medium code_review_graph/uncertainty.py::_settled code_review_graph/uncertainty.py:359 no
0.65 medium code_review_graph/uncertainty.py::_staleness code_review_graph/uncertainty.py:374 no
0.60 medium code_review_graph/uncertainty.py::_untracked_sources code_review_graph/uncertainty.py:297 no
0.60 medium code_review_graph/uncertainty.py::_untracked_note code_review_graph/uncertainty.py:348 no
0.35 low code_review_graph/uncertainty.py::_is_untracked_note code_review_graph/uncertainty.py:338 no
0.35 low code_review_graph/uncertainty.py::empty_query_confidence code_review_graph/uncertainty.py:428 yes
0.35 low tests/test_uncertainty.py::_empty_query tests/test_uncertainty.py:622 yes
0.15 low tests/test_uncertainty.py::_tracked_repo tests/test_uncertainty.py:604 yes
0.05 low tests/test_uncertainty.py::test_untracked_source_defeats_the_real_absence_claim tests/test_uncertainty.py:629 (test)
0.05 low tests/test_uncertainty.py::test_untracked_note_names_the_remedy_within_budget tests/test_uncertainty.py:643 (test)

Affected execution flows

  • empty_search_confidence — criticality 0.38, 11 node(s) across 1 file(s)
  • empty_impact_confidence — criticality 0.36, 15 node(s) across 1 file(s)

Test gaps

  • code_review_graph/uncertainty.py::_untracked_sources (code_review_graph/uncertainty.py:297)
  • code_review_graph/uncertainty.py::_is_untracked_note (code_review_graph/uncertainty.py:338)
  • code_review_graph/uncertainty.py::_untracked_note (code_review_graph/uncertainty.py:348)
  • code_review_graph/uncertainty.py::_settled (code_review_graph/uncertainty.py:359)
  • code_review_graph/uncertainty.py::_staleness (code_review_graph/uncertainty.py:374)

Token savings: this graph-backed report used ~6,064 fewer tokens (~55%) than reading every changed file in full (estimated, chars/4 approximation).


Powered by code-review-graph — local-first analysis; no code leaves the CI runner.

… count its room

The review report on this PR listed _untracked_sources, _is_untracked_note,
_untracked_note, _settled and _staleness as untested: the query-level tests
prove the sentence reaches the agent, not each helper's own contract. Eleven
tests now do — suffix calibration, the zero-not-raise paths for a missing
git and a non-repository, the predicate, the pass-through and withdrawal in
_settled, and the order of the three signals in _staleness.

Writing them found a defect. The note's suffix was exactly MAX_CONFIDENCE_CHARS
long, which left the count a budget of zero, and _fragment clipped it: 12
untracked files printed as "1~ source file(s)" — the one number the docstring
calls the actionable part, wrong, inside a sentence about accuracy. The
wording is 128 characters now, twelve digits of room, and the test asserts
the note starts with the count for 1, 12, 1234 and 999999.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants