Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 89 additions & 3 deletions code_review_graph/uncertainty.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import logging
import os
import re
import subprocess
from dataclasses import dataclass
from datetime import datetime
from pathlib import Path
Expand All @@ -46,6 +47,10 @@
IMPACT_PATTERN = "impact_radius"

_UPDATE_HINT = "run `code-review-graph update`"

# Same budget, and the same knob, incremental.py gives its git calls. This one
# only runs on an empty result, never on the hot path.
_GIT_TIMEOUT = int(os.environ.get("CRG_GIT_TIMEOUT", "30")) # seconds
_WHITESPACE = re.compile(r"\s+")


Expand Down Expand Up @@ -289,6 +294,83 @@ def _live_git_head(root: Path) -> str | None:
return _read_live_git_head(root)


def _untracked_sources(root: Path, store: GraphStore) -> int:
"""Count source files git has never seen, which the index cannot contain.

``update`` discovers changes through git, so a file that was never added
is absent from the graph however current the build is. Neither signal in
``_staleness`` can see it: the build commit still matches HEAD, and a file
the graph holds no row for has no mtime to compare against. The gap is
widest exactly when it matters most, because the files an agent has just
written are the ones it is about to ask about.

"Source" is calibrated from the graph rather than a fixed extension list,
so a repo whose languages this build does not parse never reports a file
the index was never going to hold. ``--porcelain`` already honours
``.gitignore``, so build output and vendored trees stay out.
"""
indexed_suffixes = {Path(f).suffix for f in store.get_all_files() if Path(f).suffix}
if not indexed_suffixes:
return 0
try:
result = subprocess.run(
["git", "status", "--porcelain=v1", "-z", "--untracked-files=all"],
capture_output=True,
cwd=str(root),
timeout=_GIT_TIMEOUT,
stdin=subprocess.DEVNULL,
)
except (FileNotFoundError, OSError, subprocess.SubprocessError):
return 0
if result.returncode != 0:
return 0
count = 0
for record in result.stdout.split(b"\0"):
# "?? <path>": only the untracked ones; everything else is already
# reachable through the diff that drives an incremental update.
if not record.startswith(b"?? ") or len(record) <= 3:
continue
if Path(os.fsdecode(record[3:])).suffix in indexed_suffixes:
count += 1
return count


def _is_untracked_note(note: str) -> bool:
"""Distinguish the untracked signal from the two commit/mtime ones.

An unresolved target on a stale graph is rewritten into wording about the
target; that rewrite would drop the file count, which is the actionable
part here.
"""
return "untracked by git" in note


def _untracked_note(count: int) -> str:
"""Say the working tree holds source the index was never offered."""
return _interpolated(
"",
str(count),
" source file(s) are untracked by git and absent from the index, so "
"this 0 may be incomplete; `git add` them, then "
"`code-review-graph update`",
)


def _settled(root: Path, store: GraphStore, verified: bool) -> tuple[str | None, bool]:
"""Last check before a caller is allowed to treat the graph as current.

Reached from both places where the commit and mtime signals find nothing
wrong, including the early return taken when a graph carries no build
timestamp. Untracked source is the only remaining way for the index to be
missing content, so it has to be answered on every such path rather than
on the one that happens to have metadata.
"""
untracked = _untracked_sources(root, store)
if untracked:
return _untracked_note(untracked), False
return None, verified


def _staleness(
store: GraphStore, root: Path, file_path: str | None,
) -> tuple[str | None, bool]:
Expand All @@ -312,7 +394,7 @@ def _staleness(

built_at_raw = store.get_metadata("last_updated")
if not built_at_raw or not file_path:
return None, commit_verified and not file_path
return _settled(root, store, commit_verified and not file_path)
try:
# Graphs store absolute or repo-relative paths depending on how they
# were built, so anchor relative ones rather than stat-ing the CWD.
Expand All @@ -332,7 +414,10 @@ def _staleness(
path.name,
f" changed after the last build; {_UPDATE_HINT}",
), False
return None, commit_verified
# Last, so every message this function already produced is unchanged. It
# only fires where the two checks above found nothing wrong — which is
# precisely where the caller would otherwise claim a real absence.
return _settled(root, store, commit_verified)


# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -363,7 +448,8 @@ def empty_query_confidence(
)
stale, _unused = _staleness(store, root, None)
if stale:
return _bounded(unresolved_stale_note(target))
return _bounded(stale if _is_untracked_note(stale)
else unresolved_stale_note(target))
return _bounded(not_indexed_note(target))

stale, current = _staleness(store, root, getattr(node, "file_path", None))
Expand Down
112 changes: 112 additions & 0 deletions tests/test_uncertainty.py
Original file line number Diff line number Diff line change
Expand Up @@ -553,3 +553,115 @@ def test_resolved_target_with_results_still_has_no_marker(repo):

assert result["result_count"] >= 1
assert "confidence" not in result


# ---------------------------------------------------------------------------
# The zero that only looks honest: source git has never seen
#
# An incremental update discovers changes through git, so a file that was
# never added is absent from the index however current the build commit is.
# Neither staleness signal can see it, which leaves the strongest sentence in
# this module -- "a real absence" -- attached to a zero whose only missing
# consumers are the files the agent has just written.
# ---------------------------------------------------------------------------


def _tracked_repo(root: Path) -> None:
"""Replace the fixture's placeholder .git with a real repository.

Staging is enough: ``git status --porcelain`` reports an added file as
``A``, not ``??``, so nothing here needs a commit -- or an identity to
make one with.
"""
import shutil
import subprocess

shutil.rmtree(root / ".git")
for args in (["init", "-q"], ["add", "auth.py", "main.py"]):
subprocess.run(
["git", *args], cwd=str(root), check=True,
stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
)


def _empty_query(repo: Path) -> dict:
auth = (repo / "auth.py").as_posix()
return query_graph(
pattern="inheritors_of", target=f"{auth}::login", repo_root=str(repo),
)


def test_untracked_source_defeats_the_real_absence_claim(repo):
"""Unseen source must withdraw the claim, and say how many files."""
_tracked_repo(repo)
(repo / "new_consumer.py").write_text("import auth\n", encoding="utf-8")

result = _empty_query(repo)

assert result["result_count"] == 0
confidence = result["confidence"]
assert "real absence" not in confidence
assert "untracked by git" in confidence
assert "1 source file" in confidence


def test_untracked_note_names_the_remedy_within_budget(repo):
"""A marker an agent cannot act on is only noise."""
_tracked_repo(repo)
(repo / "new_consumer.py").write_text("import auth\n", encoding="utf-8")

confidence = _empty_query(repo)["confidence"]

assert "git add" in confidence
assert len(confidence) <= MAX_CONFIDENCE_CHARS


def test_untracked_non_source_is_not_counted(repo):
"""Only extensions the graph actually indexes may raise the signal."""
_tracked_repo(repo)
(repo / "NOTES.md").write_text("scratch\n", encoding="utf-8")

confidence = _empty_query(repo)["confidence"]

assert "untracked" not in confidence
assert "is indexed" in confidence


def test_gitignored_source_is_not_counted(repo):
"""--porcelain honours .gitignore, so build output stays silent."""
_tracked_repo(repo)
(repo / ".gitignore").write_text("build/\n", encoding="utf-8")
(repo / "build").mkdir()
(repo / "build" / "generated.py").write_text("x = 1\n", encoding="utf-8")

confidence = _empty_query(repo)["confidence"]

assert "untracked" not in confidence


def test_untracked_source_adds_no_marker_to_a_nonempty_result(repo):
"""Token budget guard: the new signal stays empty-result-only."""
_tracked_repo(repo)
(repo / "new_consumer.py").write_text("import auth\n", encoding="utf-8")

result = query_graph(
pattern="callers_of",
target=f"{(repo / 'auth.py').as_posix()}::login",
repo_root=str(repo),
)

assert result["result_count"] >= 1
assert "confidence" not in result


def test_untracked_source_outranks_the_not_indexed_wording(repo):
"""An unresolved target with unseen source is a git problem, not a gap."""
_tracked_repo(repo)
(repo / "new_consumer.py").write_text("import auth\n", encoding="utf-8")

result = query_graph(
pattern="callers_of", target="NoSuchSymbol", repo_root=str(repo),
)

assert "untracked by git" in result["confidence"]
assert "not indexed" not in result["confidence"]