Skip to content
Merged
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
10 changes: 7 additions & 3 deletions harness/selectivity.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,20 +106,22 @@ def main():
sha]).stdout.strip()
rows.append((sha, desc[:50], len(selected), total,
result["recall_degraded"],
len(result["unresolved_journeys"]), selected))
len(result["unresolved_journeys"]), selected,
len(result.get("closure_confined", []))))
wtlib.remove(args.bare, wt)
finally:
shutil.rmtree(tmp, ignore_errors=True)

print(f"\n=== testgraph selectivity — {os.path.basename(args.registry)} "
f"({total} journeys), {len(shas)}-commit sweep, per-commit index ===")
scored = [r for r in rows if len(r) == 7]
scored = [r for r in rows if len(r) == 8]
for r in rows:
if len(r) == 3:
print(f" {r[0][:10]} {r[1]}: {r[2]}")
continue
sha, desc, n_sel, tot, degraded, unresolved, selected = r
sha, desc, n_sel, tot, degraded, unresolved, selected, confined = r
flag = " RECALL_DEGRADED" if degraded else ""
flag += f" CLOSURE_CONFINED({confined})" if confined else ""
unres = f" ({unresolved} journey(s) not yet in this commit's index)" if unresolved else ""
print(f"\n {sha[:10]} {desc}")
print(f" selected : {n_sel}/{tot} {selected}{flag}{unres}")
Expand All @@ -130,6 +132,7 @@ def main():
return 1
counts = [r[2] for r in scored]
degrades = sum(1 for r in scored if r[4])
confines = sum(1 for r in scored if r[7])
mean_sel = sum(counts) / len(counts)
avoided_pct = 100 * (1 - mean_sel / total)
le2 = sum(1 for c in counts if c <= 2)
Expand All @@ -143,6 +146,7 @@ def main():
print(f" all/most (>= {total - 1}) : {most} of {len(scored)}"
f" (all {total}: {all_n})")
print(f" recall_degraded fired : {degrades} of {len(scored)}")
print(f" closure_confined fired : {confines} of {len(scored)}")
# Full histogram — nothing bucketed away, so a reader can re-derive any
# threshold the two headline buckets above don't happen to answer.
hist = {}
Expand Down
47 changes: 42 additions & 5 deletions testgraph/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,47 @@ def file_node_id(conn, file_path):
return row[0] if row else None


def _load_id_temp_table(conn, table_name, ids):
"""(Re)fill a single-column TEMP TABLE with `ids`, for callers that need
to join or filter against a large id set. A raw `IN (?,?,...)` with one
bound placeholder per id hits SQLite's bound-parameter ceiling (999 on
some packaged builds) once a closure fans out wide enough; a temp table
has no such limit. Shared by `impacted_closure` and `closure_files` so
that fix applies once, not per copy.
"""
conn.execute(f"CREATE TEMP TABLE IF NOT EXISTS {table_name}(id TEXT PRIMARY KEY)")
conn.execute(f"DELETE FROM {table_name}")
conn.executemany(
f"INSERT OR IGNORE INTO {table_name}(id) VALUES (?)", [(i,) for i in ids]
)


def closure_files(conn, node_ids):
"""Distinct `file_path` values for a set of node ids (file-kind nodes
counted by their own path). Used to detect a closure that never leaves
the file its seeds started in — an edge-resolution blind spot distinct
from an unmapped seed (issue #63): the seeds resolved fine, they just
have no outbound reach on record.

Returns `None`, not a smaller set, if any id has no matching `nodes` row.
`edges` can name an id `nodes` has no row for (a dangling edge — the same
kind of drift `integrity.content_drift` exists elsewhere to catch); a
plain `WHERE id IN (...)` join silently drops such an id, which would
otherwise read identically to "this id resolves to no other file" and
manufacture a false confinement signal out of an untrustworthy index.
"""
node_ids = list(node_ids)
if not node_ids:
return set()
_load_id_temp_table(conn, "_closure_ids", node_ids)
rows = list(conn.execute(
"SELECT id, file_path FROM nodes WHERE id IN (SELECT id FROM _closure_ids)"
))
if len({r[0] for r in rows}) != len(set(node_ids)):
return None
return {r[1] for r in rows}


def impacted_closure(conn, seed_ids):
"""Transitive reverse-reachability closure of `seed_ids`, as
`{node_id: confidence}`.
Expand All @@ -104,11 +145,7 @@ def impacted_closure(conn, seed_ids):
"""
if not seed_ids:
return {}
conn.execute("CREATE TEMP TABLE IF NOT EXISTS _seeds(id TEXT PRIMARY KEY)")
conn.execute("DELETE FROM _seeds")
conn.executemany(
"INSERT OR IGNORE INTO _seeds(id) VALUES (?)", [(s,) for s in seed_ids]
)
_load_id_temp_table(conn, "_seeds", seed_ids)
kinds = ",".join("'%s'" % k for k in REACH_KINDS) # constants, safe to inline
edge_conf = (
f"MIN(COALESCE(json_extract(e.metadata, '$.confidence'), "
Expand Down
19 changes: 19 additions & 0 deletions testgraph/hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

MAX_JOURNEYS = 8
MAX_WARNINGS = 3
MAX_CONFINED = 3

# Kept as an alias: `ledger` now owns where state lives, but this name is the
# one the tests and the docs already point at.
Expand Down Expand Up @@ -78,6 +79,23 @@ def render(result, repo, more_cmd=None):

if result.get("recall_degraded"):
lines.append(" RECALL DEGRADED — unbounded impact, all journeys listed")
# Like recall_degraded above, this gets its own line rather than riding
# the warnings channel below: capped at MAX_WARNINGS, a push with an
# unapproved-registry warning plus entry drift already queued ahead of it
# would silently swallow the one signal issue #63 exists to surface. But
# NEVER just swallowed, not the same as UNCAPPED IN LENGTH: a wide
# rename/refactor confining many files would otherwise print one
# unbroken multi-hundred-character line, the exact noise rule 2 in this
# module's docstring exists to prevent.
confined = result.get("closure_confined", [])
if confined:
shown = ", ".join(confined[:MAX_CONFINED])
if len(confined) > MAX_CONFINED:
shown += f", … {len(confined) - MAX_CONFINED} more"
lines.append(
f" NOTE: impact for {shown} did not leave the file it started "
f"in — that file's own contribution is UNKNOWN, not verified-safe"
)
# Warnings are the channel that carries an unapproved registry and entry
# drift — both mean "this answer may be understated", so they are worth the
# lines. Capped: an unbounded warning block is the noise problem again.
Expand Down Expand Up @@ -141,6 +159,7 @@ def run(repo, base, head, registry_path=None, caller="pre-push"):
record["n_journeys"] = len(result.get("journeys", []))
record["journey_ids"] = [j["id"] for j in result.get("journeys", [])]
record["recall_degraded"] = bool(result.get("recall_degraded"))
record["closure_confined"] = result.get("closure_confined", [])
record["duration_ms"] = int((time.time() - started) * 1000)
more = (
f"python3 -m testgraph.select --repo {repo} --base {base} --head {head}"
Expand Down
102 changes: 99 additions & 3 deletions testgraph/select.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,15 +226,27 @@ def select(repo, base, head, db_path, registry_path, strict_registry=True):
# PRODUCT_EXT but not in this repo's graph. Per-file node sets, not one
# running total, so one mapped file cannot mask an unmapped one.
unmapped = []
unmapped_files = set()

def _mark_unmapped(path, detail):
# `unmapped_files` gates the confinement check below (an untrusted
# seed must not read as evidence of confinement) — one helper instead
# of three independent append/add pairs means that gate can't drift
# out of sync with `unmapped` itself.
unmapped.append(f"{path} ({detail})")
unmapped_files.add(path)

seeds = set()
seeds_by_file = {}
for f, rs in sorted(ranges.items()):
in_file = set()
for lo, hi in rs:
in_file.update(dbmod.nodes_for_lines(conn, f, lo, hi))
if in_file:
seeds.update(in_file)
seeds_by_file[f] = in_file
else:
unmapped.append(f"{f} (changed lines map to no indexed symbol)")
_mark_unmapped(f, "changed lines map to no indexed symbol")

# Whole-file changes (deletions, renames) have no line ranges to map: seed
# every symbol the file contains. A file deleted in `head` is usually absent
Expand All @@ -246,8 +258,19 @@ def select(repo, base, head, db_path, registry_path, strict_registry=True):
nodes = dbmod.nodes_in_file(conn, path)
if nodes:
seeds.update(nodes)
# A rename that ALSO carries edited hunks lands in both `ranges`
# and `whole_files` for the new path. `seeds` above always gets
# the full file (recall-first: a rename changes the module path
# for every importer, so the whole file is in play regardless of
# which lines moved) — but for the confinement check specifically,
# letting the broader whole-file set clobber a precise range-based
# entry can mask a real issue-#63 confinement in the lines that
# actually changed behind unrelated untouched symbols that happen
# to reach elsewhere. Keep the narrower, already-set entry.
if path not in seeds_by_file:
seeds_by_file[path] = set(nodes)
else:
unmapped.append(f"{path} ({reason})")
_mark_unmapped(path, reason)

# A changed file that is NEWER than its index row is a third way to be
# unmappable, and the quietest. The other two resolve to no node and are
Expand All @@ -267,11 +290,70 @@ def select(repo, base, head, db_path, registry_path, strict_registry=True):
# exceptional path rather than every push.
drifted = integrity.content_drift(conn, repo, set(ranges) | set(whole_files))
for path in sorted(drifted):
unmapped.append(f"{path} (bytes differ from the indexed copy — line spans are stale)")
_mark_unmapped(path, "bytes differ from the indexed copy — line spans are stale")

impacted = dbmod.impacted_closure(conn, seeds)
entry_map = reg.resolve_entries(conn, registry)

# A closure that resolves fine but never leaves the file(s) its seeds
# started in is a second, silent way to be blind (issue #63) — distinct
# from `unmapped` above (no node at all). The seeds have symbols; those
# symbols just have no recorded outbound reach. Checked per file, not
# against the union of every seeded file in this diff, so a genuinely
# cross-file change cannot mask a same-diff file that stayed confined.
# Files already in `unmapped` are skipped: their seeds are untrusted, not
# evidence of confinement.
#
# NOT flagged when the file's own closure already lands on a registered
# entry point (`entry_map`): a route handler with no callers on record is
# not a blind spot, it's a correctly-confident answer — the seed IS the
# thing the registry names, so there is nothing "unknown" left to say.
# Reproduced against this repo's own index before this guard existed: 7
# of 11 "confined" files had already selected a journey at confidence
# 1.0, and the warning called that UNKNOWN anyway.
#
# One recursive traversal per seeded file (beyond the single-file reuse
# below), and `closure_files` scans every resulting node rather than
# short-circuiting on the first one outside `f`: a mechanical
# rename/refactor touching many files pays for it on every one. Accepted
# for the same reason `journeys` already loops `caller_edge_count` per
# entry above — this selector is recall-first and already spends
# per-item DB round trips elsewhere; a diff wide enough to feel this is
# also wide enough to be a whole-file/`unmapped` case on the commonest
# paths. Revisit if this ever shows up in profiling.
confined_files = []
for f, file_seeds in sorted(seeds_by_file.items()):
if f in unmapped_files:
continue
# Judged per SEED, not per file: a file can carry both a registered
# entry (correctly confident on its own) and an unrelated edited
# symbol that is the actual blind spot. Clearing the whole file
# because ANY of its seeds happens to be an entry point silently
# swallowed the NOTE for the seed that needed it — reproduced with
# two unrelated edited symbols in one file, one an entry, one not.
uncovered = file_seeds - entry_map.keys()
if not uncovered:
continue
# `impacted` is already this closure whenever the uncovered seeds are
# the full seed set — reuse it instead of re-running the same
# recursive traversal. Checked by value, not by file/seed counts:
# inferring it silently breaks if a future seed source desyncs from
# `seeds`.
file_impacted = (
impacted if uncovered == seeds else dbmod.impacted_closure(conn, uncovered)
)
# `file_impacted` always contains at least the seeds themselves
# (`impacted_closure` seeds every id at 1.0), and every seed in
# `uncovered` came from a `nodes` row whose own `file_path == f` — so
# `closure_files` here can never come back empty, UNLESS the closure
# reached an id with no matching `nodes` row (a dangling edge / stale
# index), which `closure_files` reports as `None` rather than
# silently reading as "resolves to no other file". An index
# inconsistent enough to produce that is not evidence of anything.
reached_files = dbmod.closure_files(conn, file_impacted.keys())
if reached_files is not None and reached_files <= {f}:
confined_files.append(f)

touched = {}
for nid in impacted.keys() & set(entry_map):
for jid in entry_map[nid]:
Expand Down Expand Up @@ -318,11 +400,18 @@ def select(repo, base, head, db_path, registry_path, strict_registry=True):

journeys.sort(key=lambda j: (-j["rank"], reg.journey_sort_key(j["id"])))

# Not appended to `warnings`: this is its own signal, deliberately kept
# off the capped, generic channel. `_render` below and `hook.render` each
# give it a dedicated, uncapped line (`closure_confined` on the result is
# the data both read from) — riding `warnings` would let it silently drop
# off a push whose other warnings already filled hook.py's MAX_WARNINGS.

result.update(
status="OK",
changed_files=sorted(ranges),
whole_file_changes=whole_files,
recall_degraded=bool(unmapped),
closure_confined=confined_files,
seed_symbols=len(seeds),
impacted_symbols=len(impacted),
journeys=journeys,
Expand Down Expand Up @@ -354,6 +443,13 @@ def _render(result):
lines.append(f" whole-file: {path} ({reason})")
if result.get("recall_degraded"):
lines.append(" RECALL DEGRADED — unbounded impact, all journeys listed")
for f in result.get("closure_confined", []):
lines.append(
f" NOTE: impact for {f} did not leave the file it started in — "
f"either the module is genuinely leaf-only, or its callers are "
f"not linked in the index; that file's own contribution is "
f"UNKNOWN, not verified-safe"
)
if not result["journeys"]:
lines.append("journeys to test: NONE (no product-behavior change detected)")
else:
Expand Down
Loading
Loading