From 5e95478a2ce03b3715c635191021a4affaf2b2ed Mon Sep 17 00:00:00 2001 From: Tim Stranske Date: Sat, 22 Aug 2026 22:23:43 -0500 Subject: [PATCH] fix(advisor): a suppressed surface must actually be quiet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by the first two real audit runs against this system, independently, in the same session. `NO_BINDING` suppressed only the DECLARED half. So `repo-audit:phase-1` — whose entire point is that the playbook says "Orient (bash only, NO agents)" — returned `binding_for() == {}` as designed while `advise()` still offered `deliberate-break-verifier` and `testgen-lane` from the keyword classifier. Two agent capabilities recommended at a phase defined by having no agents. The Workflows audit put it more precisely than I had: the deliberately-empty binding was "byte-identical to a classification failure". capability_advisor declares NO_BINDING with a rationale, says in its own comment that silent absence and deliberate emptiness "must not look alike -- that is this repo's founding defect", and then emitted none of it: phase-1 and a control consult against a nonexistent surface returned the identical `reason`. The vocabulary already existed for the other direction ("N capability(ies) are DECLARED..."). Suppression now covers the whole answer and says why: confidence `"suppressed"`, empty `capabilities`, and a reason naming the surface and quoting its declared rationale. "No agents here" is a statement about the CONTEXT, not about one code path. MY SELFTEST ASSERTED THE WRONG LAYER. It checked `binding_for(surface) == {}` -- the table -- and never what a CALLER receives. That is exactly the gap I have been catching in other people's work this session. It now asserts on `advise()`: empty capabilities, `confidence == "suppressed"`, and a reason that explains the emptiness. Break->revert: reverting suppression to the binding layer fails with the leaked `unbound-testgen` candidate printed in full. verify.py unchanged otherwise; local suite still shows the three pre-existing evidence-acquisition failures. Co-Authored-By: Claude Opus 5 --- capability_advisor.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/capability_advisor.py b/capability_advisor.py index 1e80c66..8a42322 100644 --- a/capability_advisor.py +++ b/capability_advisor.py @@ -216,6 +216,27 @@ def advise(text: str, *, repository: str = "", lane: str = "opener", skill: str call a pure query (used by tests and by dry inspection). """ caps = capabilities.load(path or capabilities.REG) + + # A SUPPRESSED SURFACE MUST BE ACTUALLY QUIET. `NO_BINDING` used to suppress only the DECLARED + # half, so `repo-audit:phase-1` — whose whole point is that the playbook says "Orient (bash only, + # NO agents)" — still offered `deliberate-break-verifier` and `testgen-lane` from the keyword + # classifier. Found by the first real audit run against this system: "an empty-by-design surface + # is not actually quiet". The selftest missed it because it asserted `binding_for(...) == {}` — + # the binding — and never what a CALLER receives. Suppression now covers the whole answer, + # because "no agents here" is a statement about the context, not about one code path. + suppressed = binding_suppressed(surface) if surface else "" + if suppressed: + return { + "task": text, "experiment_id": experiment_id(text), + "useful": False, "confidence": "suppressed", "skill": skill or None, + "surface": surface, "repository": repository, + "task_types": [], "capabilities": [], "dispatch_ready_count": 0, + "bound_count": 0, "bound_capabilities": [], "not_applicable": [], + "coverage": {"ledger_count": len(caps), "matched": 0, "not_applicable": 0, + "by_entry_mode": {}}, + "reason": f"surface {surface!r} deliberately takes no capabilities: {suppressed}", + } + candidates = classify_task(text) if not candidates: # A DECLARED BINDING MUST SURVIVE A CLASSIFICATION MISS. This early return used to drop @@ -714,6 +735,11 @@ def binding_for(surface: str, *, path=None) -> dict[str, str]: return out +def suppressed_reason_in(advice: dict) -> bool: + """Does this advice explain that its surface is deliberately empty?""" + return "deliberately takes no capabilities" in str(advice.get("reason") or "") + + def binding_suppressed(surface: str) -> str: """Why this surface deliberately binds nothing, or '' if it is not suppressed. @@ -1101,6 +1127,13 @@ def _selftest_bindings() -> None: try: assert sorted(binding_for("t-proc:p2")) == ["bound-a", "bound-b"], "phase must merge" assert binding_for("t-proc:p1") == {}, "NO_BINDING must suppress inheritance" + # AND THE WHOLE ANSWER, not just the declared half. Asserting only the binding is + # what let phase-1 keep offering classifier matches at a bash-only phase. + quiet = advise("add unit tests for the retry helper", surface="t-proc:p1", + path=ledger, record=False) + assert quiet["capabilities"] == [], quiet["capabilities"] + assert quiet["confidence"] == "suppressed", quiet["confidence"] + assert quiet["useful"] is False and suppressed_reason_in(quiet), quiet["reason"] assert binding_suppressed("t-proc:p1"), "and must SAY why it is empty" assert not binding_suppressed("t-proc:p2") # An unknown phase of a known surface still gets the surface-wide set.