diff --git a/src/scribe/mcp/server.py b/src/scribe/mcp/server.py index f13f96b..22d8111 100644 --- a/src/scribe/mcp/server.py +++ b/src/scribe/mcp/server.py @@ -119,7 +119,7 @@ _READ_ONLY_TOOLS = frozenset({ "list_repo_bindings", # The shape ledger's todo query (#2789). Reads only — classify_shapes is # the write, and it is deliberately NOT here. - "list_shapes", "shape_history", + "list_shapes", "shape_history", "stamps_to_review", # The retrieval telemetry readout (#2975). Aggregates two log tables and # writes nothing. Listed explicitly because its name carries no read # prefix, so the completeness test below cannot derive it — the same diff --git a/src/scribe/mcp/tools/shapes.py b/src/scribe/mcp/tools/shapes.py index c1d5dd3..68a93a7 100644 --- a/src/scribe/mcp/tools/shapes.py +++ b/src/scribe/mcp/tools/shapes.py @@ -247,6 +247,42 @@ async def shape_history( ) +async def stamps_to_review(project_id: int, top: int = 10) -> dict: + """Judged rows whose evidence no longer meets the ledger's bar, and canons + whose own rows no longer agree what they are. **Read-only — you decide.** + + This tool deliberately cannot fix anything. Every row it lists was written + unattended by the write-path hook on a similarity score, and what made + that harmful was not one bad score but a machine recording a permanent + classification with nobody reading it. Un-asserting them automatically + would be the same mistake with a wider blast radius. So: read the + evidence, judge, and record the judgment yourself with `classify_shapes` + — under your own name, with a reason. + + `weak` — rows the hook stamped on a resemblance below the current floor + (`floor` in the response). The floor arrived after they did, and a guard + at the point of classification never undoes what is already stored. Rows + an agent or an audit judged are NOT listed at any age: a judgment is not + weak evidence, it is a different kind of evidence. Each row carries its + score, signature and derived form so you can judge rather than trust the + threshold. + + `incoherent` — canons whose judged rows do not agree on a form: the + `forms` histogram, how many members were hook-stamped weakly, and a + sample. A canon asserts that some shapes are the same sort of thing; when + its members are a class, three getters and a dozen tests, that assertion + has stopped being true and every base-rate reading built on it — the + divergence prompt included — is reading noise. + + A row listed here is a question, not a verdict. Some will be correct. + + Read-only; requires read access to the project. + """ + uid = current_user_id() + rows = await shape_ledger_svc.live_rows_for(uid, project_id) + return shape_ledger_svc.stamps_to_review(rows, top=top) + + async def confirm_shape_proposals( project_id: int, snippet_id: int = 0, @@ -326,5 +362,6 @@ def register(mcp) -> None: for fn in ( classify_shapes, classify_shapes_by_rule, list_shapes, refresh_pattern_coverage, confirm_shape_proposals, shape_history, + stamps_to_review, ): mcp.tool(name=fn.__name__)(fn) diff --git a/src/scribe/services/shape_ledger.py b/src/scribe/services/shape_ledger.py index 61d7e79..cab9635 100644 --- a/src/scribe/services/shape_ledger.py +++ b/src/scribe/services/shape_ledger.py @@ -439,6 +439,16 @@ async def mark_canonicals( await session.commit() +async def live_rows_for(user_id: int, project_id: int) -> list[CodeShape]: + """`live_rows` behind the project read gate — for callers that arrive from + outside (an MCP tool, a route) rather than from a job that already + established who is asking. Empty for a project the caller cannot read, + never a partial answer.""" + if not await access.can_read_project(user_id, project_id): + return [] + return await live_rows(project_id) + + async def live_rows(project_id: int) -> list[CodeShape]: """Every un-vanished ledger row for a project — the accounting readout's input, across ALL its repos (a repo unreachable this refresh still counts; @@ -1137,6 +1147,34 @@ async def recent_pulls(user_id: int, *, window: timedelta = PULL_WINDOW) -> dict return {} +# THE HOOK'S EVIDENCE, WRITTEN ONCE AND READ BACK BY ONE PARSER. +# +# The stamp's score used to exist only inside a prose sentence, which meant +# nothing could ask "how strong was the evidence for this row?" without +# re-deriving it. That is how 32 rows on one project and 334 on another sat +# unexamined: the number was printed, never stored, never queried. Format and +# parser live beside each other so a change to one that breaks the other is a +# visible edit rather than a silent drift (rule 33's reasoning, one module +# down). +_REFERENCE_REASON = "hook: pulled #{sid}; payload references `{symbol}`" +_RESEMBLE_REASON = "hook: pulled #{sid}; payload resembles it ({score:.2f})" +_RESEMBLE_READ = re.compile( + r"^hook: pulled #(\d+); payload resembles it \(([0-9]*\.?[0-9]+)\)$" +) + + +def stamp_score(reason: str | None) -> float | None: + """The resemblance a hook stamp recorded, or None if it was not one. + + None means "this row was not written on a similarity score" — a by-name + reference, an agent judgment, an audit — NOT "the score was zero". Every + caller treats the two differently, because a row a person judged is not + weak evidence, it is a different kind of evidence entirely. + """ + m = _RESEMBLE_READ.match((reason or "").strip()) + return float(m.group(2)) if m else None + + def _stamp_allowed(rank: int, mine: str, canon: str) -> bool: """May this evidence assert that a shape of form ``mine`` IS ``canon``? @@ -1208,9 +1246,11 @@ async def stamp_write_path_instances( symbol = fields.get("symbol") or "" kind = snippet_kind(symbol, fields.get("language") or "") if references_symbol(code, symbol, kind): - rank, why = 2, f"hook: pulled #{sid}; payload references `{_norm_symbol(symbol)}`" + rank, why = 2, _REFERENCE_REASON.format( + sid=sid, symbol=_norm_symbol(symbol) + ) elif resembles.get(sid, 0.0) >= _RESEMBLE_MIN: - rank, why = 1, f"hook: pulled #{sid}; payload resembles it ({resembles[sid]:.2f})" + rank, why = 1, _RESEMBLE_REASON.format(sid=sid, score=resembles[sid]) else: # A SCORE BELOW THE FLOOR IS NOT WEAK EVIDENCE, IT IS NONE. This # branch used to be `elif sid in resembles`, which took any score @@ -2199,6 +2239,98 @@ async def flag_divergence(project_id: int, *, since: datetime | None) -> int: return flagged +# How many of a canon's rows a review listing shows before "…" — enough to +# judge from, not the whole table. +_REVIEW_ROWS_SHOWN = 12 + + +def stamps_to_review(rows: Iterable[CodeShape], *, top: int = 10) -> dict: + """Judged rows whose EVIDENCE no longer meets the bar the ledger now + holds, and canons whose own rows no longer agree what they are. + + THIS CHANGES NOTHING, AND THAT IS THE POINT. Every row here was written + unattended by the write-path hook on a similarity score, and the damage + that caused was not the score being wrong once — it was a machine + asserting a permanent classification with nobody reading it. A sweep that + silently un-asserted them would be the identical mistake with a larger + blast radius and a cleaner conscience. So this reports; an agent reads the + evidence, decides, and records the decision through `classify_shapes` + under its own name. The ledger should be able to say "these look wrong" + without being able to act on it alone. + + Two lists, because they are different questions: + + `weak` — rows stamped on a resemblance below `_RESEMBLE_MIN`. The floor + arrived after they did (#4204), and a guard at the point of classification + does not undo the classifications already stored (lesson #4202). Rows a + person or an audit judged are never listed, however old: a human judgment + is not weak evidence, it is a different kind of evidence. + + `incoherent` — canons whose judged rows do not agree on a form. A canon is + a claim that some set of shapes are the same sort of thing; when its own + members are a class, three getters and a dozen tests, that claim has + stopped being true, and every base-rate reading built on it is reading + noise. `canon_form` already makes such a canon fall silent — this is what + makes it VISIBLE, which is the half that was missing. + """ + live = [r for r in rows if r.vanished_at is None] + weak = [] + for r in live: + if r.status not in _NEEDS_TARGET or r.classified_by != "hook": + continue + score = stamp_score(r.reason) + if score is None or score >= _RESEMBLE_MIN: + continue + weak.append({ + "path": r.path, "symbol": r.symbol, "kind": r.kind, + "status": r.status, "snippet_id": r.snippet_id, + "score": score, "signature": r.signature or "", + "form": shape_form(r.signature or "", r.kind), + "classified_at": r.classified_at.isoformat() if r.classified_at else None, + }) + weak.sort(key=lambda d: (d["score"], d["path"], d["symbol"])) + + by_canon: dict[int, list[CodeShape]] = {} + for r in live: + if r.status in _NEEDS_TARGET and r.snippet_id: + by_canon.setdefault(int(r.snippet_id), []).append(r) + incoherent = [] + for sid, members in by_canon.items(): + forms: dict[str, int] = {} + for r in members: + f = shape_form(r.signature or "", r.kind) + if f: + forms[f] = forms.get(f, 0) + 1 + readable = sum(forms.values()) + if readable < _DENSITY_MIN_JUDGED: + continue # too few to say anything either way + if canon_form(members, sid): + continue # a form holds the majority: coherent + incoherent.append({ + "snippet_id": sid, + "judged": len(members), + "forms": dict(sorted(forms.items(), key=lambda kv: -kv[1])), + "weak_rows": sum( + 1 for r in members + if r.classified_by == "hook" + and (stamp_score(r.reason) or 1.0) < _RESEMBLE_MIN + ), + "sample": [ + {"path": r.path, "symbol": r.symbol, + "signature": r.signature or "", "by": r.classified_by} + for r in members[:_REVIEW_ROWS_SHOWN] + ], + }) + incoherent.sort(key=lambda d: (-d["weak_rows"], -d["judged"])) + return { + "weak_count": len(weak), + "weak": weak[:top], + "incoherent_count": len(incoherent), + "incoherent": incoherent[:top], + "floor": _RESEMBLE_MIN, + } + + def divergence_summary(rows: Iterable[CodeShape], *, top: int = 10) -> dict: """Readout view: flagged shapes (newest first) and the recheck count.""" flagged = [r for r in rows if r.diverges_from is not None and r.status in _MECHANICAL_TODO] diff --git a/tests/test_stamps_to_review.py b/tests/test_stamps_to_review.py new file mode 100644 index 0000000..115dafc --- /dev/null +++ b/tests/test_stamps_to_review.py @@ -0,0 +1,201 @@ +"""The review surface: rows whose evidence no longer meets the bar, and +canons whose own rows no longer agree what they are (#4204, #4208). + +WHAT THESE GUARD, and it is a property rather than an output shape. The +ledger was poisoned by a machine writing permanent classifications that +nobody read — 32 rows on one project, 334 on another. The correction is NOT +a machine that un-writes them; that is the same mistake pointed the other +way. It is a reader that puts the evidence in front of a judge. + +So the tests below assert two things hardest: + + * a judgment is NEVER listed as weak, however old and whatever its score + would have been — an agent's decision is a different kind of evidence, + not a worse one; + * the listing changes nothing, which is checked by giving it rows and + asserting the rows come back untouched. + +The round-trip test exists because the score used to live only inside a +prose sentence. Writer and parser are now one constant apart, and a change +to one that the other does not follow is exactly the kind of silent rot the +hook's own history is made of. +""" + + + + +import pytest + +from scribe.services.shape_ledger import ( + _RESEMBLE_MIN, _RESEMBLE_REASON, stamp_score, stamps_to_review, +) + + +class _Row: + """A ledger row, as the review reader touches one.""" + + def __init__(self, *, path="src/a.py", symbol="f", kind="sym", + status="instance", snippet_id=1, reason=None, + classified_by="hook", signature="async def f():", + vanished_at=None, classified_at=None): + self.path, self.symbol, self.kind = path, symbol, kind + self.status, self.snippet_id, self.reason = status, snippet_id, reason + self.classified_by, self.signature = classified_by, signature + self.vanished_at, self.classified_at = vanished_at, classified_at + + +def _stamped(score, **kw): + return _Row(reason=_RESEMBLE_REASON.format(sid=1, score=score), **kw) + + +# ── the writer and the parser are one constant apart ────────────────────── + +@pytest.mark.parametrize("score", [0.0, 0.5, 0.69, 0.77, 0.8, 0.95, 1.0]) +def test_every_score_the_hook_can_write_reads_back(score) -> None: + written = _RESEMBLE_REASON.format(sid=4204, score=score) + assert stamp_score(written) == pytest.approx(round(score, 2)) + + +def test_the_real_reasons_from_the_poisoned_ledgers_parse() -> None: + """Verbatim from Portal and Scribe rows, so the parser is pinned to text + that actually exists in the database rather than to text it generates.""" + assert stamp_score("hook: pulled #3283; payload resembles it (0.69)") == 0.69 + assert stamp_score("hook: pulled #3283; payload resembles it (0.77)") == 0.77 + assert stamp_score("hook: pulled #2860; payload resembles it (0.68)") == 0.68 + + +@pytest.mark.parametrize("reason", [ + None, "", " ", + "hook: pulled #3461; payload references `icon-btn`", # the OTHER evidence + "an agent wrote prose here", + "hook: pulled #1; payload resembles it", # no score +]) +def test_anything_that_is_not_a_score_reads_as_no_score(reason) -> None: + """None means "not written on a score", never "scored zero" — the two + must not collapse, because one is a judgment and the other is weak.""" + assert stamp_score(reason) is None + + +# ── weak: only hook stamps, only below the floor ────────────────────────── + +def test_a_stamp_below_the_floor_is_listed_with_its_evidence() -> None: + out = stamps_to_review([_stamped(0.69)]) + assert out["weak_count"] == 1 + row = out["weak"][0] + assert row["score"] == 0.69 + assert row["form"] == "async-fn" + assert row["signature"] == "async def f():" + assert out["floor"] == _RESEMBLE_MIN + + +def test_a_stamp_at_or_above_the_floor_is_not_listed() -> None: + assert stamps_to_review([_stamped(_RESEMBLE_MIN)])["weak_count"] == 0 + assert stamps_to_review([_stamped(0.95)])["weak_count"] == 0 + + +@pytest.mark.parametrize("by", ["agent", "audit", "import", "mechanical"]) +def test_a_judgment_is_never_weak_however_it_would_have_scored(by) -> None: + """The measured case: 302 of Scribe's 334 rows under one canon were + `audit`-classified with no score at all. Listing those as weak would + invite an agent to withdraw the only judgments in the ledger that were + ever actually made by one.""" + rows = [_Row(classified_by=by, reason=None), + _Row(classified_by=by, + reason=_RESEMBLE_REASON.format(sid=1, score=0.10))] + assert stamps_to_review(rows)["weak_count"] == 0 + + +def test_an_unclassified_or_vanished_row_is_not_listed() -> None: + assert stamps_to_review([_stamped(0.1, status="unclassified")])["weak_count"] == 0 + assert stamps_to_review([_stamped(0.1, vanished_at="gone")])["weak_count"] == 0 + + +def test_the_weakest_evidence_is_listed_first() -> None: + out = stamps_to_review([ + _stamped(0.77, symbol="c"), _stamped(0.68, symbol="a"), + _stamped(0.71, symbol="b"), + ]) + assert [r["symbol"] for r in out["weak"]] == ["a", "b", "c"] + + +def test_top_bounds_the_listing_but_not_the_count() -> None: + out = stamps_to_review([_stamped(0.7, symbol=f"s{i}") for i in range(9)], top=3) + assert out["weak_count"] == 9 and len(out["weak"]) == 3 + + +# ── incoherent: a canon whose own members disagree ──────────────────────── + +def _member(sig, sid=7, **kw): + return _Row(snippet_id=sid, signature=sig, classified_by="audit", **kw) + + +def test_a_canon_whose_members_agree_is_not_listed() -> None: + rows = [_member("async def a():"), _member("async def b():"), + _member("async def c():"), _member("async def d():")] + assert stamps_to_review(rows)["incoherent_count"] == 0 + + +def test_a_canon_whose_members_are_all_different_things_is_listed() -> None: + """Portal's #3283, in miniature: a class, a getter, a test and a binding + recorded as instances of one pattern.""" + rows = [_member("class SessionAbsent(RuntimeError):"), + _member("def build_channel() -> str:"), + _member("async def attach(self) -> None:"), + _member("MAX = 10")] + out = stamps_to_review(rows) + assert out["incoherent_count"] == 1 + entry = out["incoherent"][0] + assert entry["snippet_id"] == 7 and entry["judged"] == 4 + assert set(entry["forms"]) == {"type", "fn", "async-fn", "binding"} + assert len(entry["sample"]) == 4 + + +def test_too_few_readable_members_says_nothing_either_way() -> None: + """Two rows that disagree is not an incoherent canon, it is two rows. + The floor is the same one the density check uses.""" + rows = [_member("class A:"), _member("def b():")] + assert stamps_to_review(rows)["incoherent_count"] == 0 + + +def test_unreadable_signatures_do_not_manufacture_incoherence() -> None: + rows = [_member(""), _member(" "), _member("???"), _member("")] + assert stamps_to_review(rows)["incoherent_count"] == 0 + + +def test_a_canon_carrying_weak_stamps_is_ranked_above_one_that_is_not() -> None: + clean = [_member("class A:", sid=1), _member("def b():", sid=1), + _member("MAX = 1", sid=1), _member("async def c():", sid=1)] + dirty = [_Row(snippet_id=2, signature=s, classified_by="hook", + reason=_RESEMBLE_REASON.format(sid=2, score=0.70)) + for s in ("class D:", "def e():", "MAX = 2", "async def f():")] + out = stamps_to_review(clean + dirty) + assert out["incoherent_count"] == 2 + assert out["incoherent"][0]["snippet_id"] == 2 + assert out["incoherent"][0]["weak_rows"] == 4 + assert out["incoherent"][1]["weak_rows"] == 0 + + +# ── the property that matters most: it is a reader ──────────────────────── + +def test_the_review_changes_nothing_about_the_rows_it_reads() -> None: + """Asserted rather than assumed. The whole design claim of this surface + is that it reports and never acts; a future refactor that "helpfully" + resets a status here would pass every other test in this file.""" + rows = [_stamped(0.69), _member("class A:"), _member("def b():"), + _member("MAX = 1"), _member("async def c():")] + before = [(r.status, r.snippet_id, r.classified_by, r.reason) for r in rows] + stamps_to_review(rows) + after = [(r.status, r.snippet_id, r.classified_by, r.reason) for r in rows] + assert before == after + + +def test_the_service_carries_no_machinery_for_bulk_withdrawal() -> None: + """A structural guard (rule 167) on the design decision, not on output. + If someone adds an auto-retire path to the ledger, this fails and the + conversation about whether a machine may un-judge in bulk happens again + — deliberately, rather than in a diff nobody read.""" + import scribe.services.shape_ledger as ledger + + body = open(ledger.__file__).read() + for banned in ("def retire_weak", "def auto_unclassify", "def bulk_withdraw"): + assert banned not in body, banned