Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
647ae2e
fix(specialist): stop shipping a proposal schema that fails its own g…
zoroyihan7 Aug 12, 2026
69b0c6a
fix(critic): make a proposal format slip advisory instead of fatal
zoroyihan7 Aug 12, 2026
d15e2b4
Merge remote-tracking branch 'origin/main' into fix/1143-proposal-schema
zoroyihan7 Aug 12, 2026
be260e1
fix(loop): hold a reject to the verdict its own rule declared
zoroyihan7 Aug 13, 2026
018911d
Merge remote-tracking branch 'origin/main' into fix/1143-proposal-schema
zoroyihan7 Aug 13, 2026
9e0c852
test(critic): drop the unused binding in the downgrade audit test
zoroyihan7 Aug 13, 2026
ea3eab2
fix(loop): read the cited rule from the verdict's prose too
zoroyihan7 Aug 13, 2026
29b11de
Merge remote-tracking branch 'origin/main' into fix/1143-proposal-schema
zoroyihan7 Aug 13, 2026
bd77bf2
fix(critic): carry the cited rule out of the runtime, not just into it
zoroyihan7 Aug 13, 2026
1095895
fix(loop): hold a reject to its rule only when that rule is the whole…
zoroyihan7 Aug 13, 2026
56cffd5
fix(loop): a held reject is not a patch-landing permit
zoroyihan7 Aug 13, 2026
a1fc118
fix(specialists): strip the gain claim that hid under the Coordinator…
zoroyihan7 Aug 13, 2026
4fb6b6c
fix(loop): read a rule citation, not a rule mention
zoroyihan7 Aug 13, 2026
f0d723c
fix(loop): hold a reject only where the cited rule has jurisdiction
zoroyihan7 Aug 13, 2026
f8a904c
fix(loop): read a citation that opens a verdict, not a line that rese…
zoroyihan7 Aug 13, 2026
645539f
fix(loop): read the grounds a verdict_map entry actually states
zoroyihan7 Aug 13, 2026
75ce69c
fix(critic): put the cited rule in the schema the Critic is handed
zoroyihan7 Aug 13, 2026
0eb25e3
test(loop): cover the verdict paths a mutation could delete unnoticed
zoroyihan7 Aug 13, 2026
51a13fd
fix(critic): name the field that fills the schema slot, not the rule
zoroyihan7 Aug 13, 2026
dd49772
test(loop): cite a rule in the prose the declared code has to outrank
zoroyihan7 Aug 13, 2026
505dfb8
test(loop): name the mechanism the citation scan actually uses
zoroyihan7 Aug 13, 2026
316684a
fix(specialists): audit the fields the strip removed, not a second scan
zoroyihan7 Aug 13, 2026
31052a5
fix(loop): read the grounds a verdict_map entry inherits from its ver…
zoroyihan7 Aug 13, 2026
83a8622
fix(loop): read a citation in every prose field the entry states
zoroyihan7 Aug 13, 2026
66e44ec
test(loop): drop a citation case no mutation kills on its own
zoroyihan7 Aug 13, 2026
d816943
refactor(specialists): leave one reader of the forbidden field list
zoroyihan7 Aug 13, 2026
3034c02
fix(loop): let the batch's grounds hold a variant's reject, never sof…
zoroyihan7 Aug 14, 2026
279299a
fix(loop): hold a variant on every ground the batch states, attributi…
zoroyihan7 Aug 14, 2026
4af8e71
fix(loop): hold a variant on every finding the batch states, counting…
zoroyihan7 Aug 14, 2026
1642e9d
Merge remote-tracking branch 'origin/main' into fix/1143-proposal-schema
zoroyihan7 Aug 16, 2026
c14ee71
Drop the unused top-level tokenize import from coordinator_helpers
zoroyihan7 Aug 16, 2026
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
14 changes: 13 additions & 1 deletion src/hyperloom/agents/critic/references/verdict_schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ caller explicitly asks for markdown.
],
"alternative_action": null,
"advice_text": "",
"notes": []
"notes": [],
"failure_reason_code": ""
}
```

Expand Down Expand Up @@ -62,6 +63,17 @@ Verdict rules:
- `needs_review`: dispatch must not proceed. Use for high-risk mock, timeout,
unavailable, or insufficient-evidence cases.

`failure_reason_code` names the review rule a non-`approve` verdict rests on,
copied verbatim from the `failure_reason_code` of the matching rule in
`judge_bundle.review_constraints` (the quantitative-claim rule, a cross-domain
rule, or a safety guard). Leave it empty when the verdict rests on your own
judgement rather than a rule handed to you in the bundle. Several of those
rules declare `advise` as their `failure_verdict` because rejecting on them
costs the round every proposal in the set; naming the rule is how the
Coordinator can tell such a verdict apart from a substantive rejection, so a
verdict that cites a rule must carry its code rather than only mentioning it in
`reasoning`.

Approve example:

```json
Expand Down
2 changes: 2 additions & 0 deletions src/hyperloom/agents/critic/runtime/decision_reviewer.py
Original file line number Diff line number Diff line change
Expand Up @@ -976,6 +976,7 @@ def _commit_coordinator_inbox(
alternative_action=item.get("alternative_action"),
advice_text=advice_text,
notes=item.get("notes") or [],
failure_reason_code=str(item.get("failure_reason_code") or ""),
)
except IntentEnvelopeValidationError as exc:
raise ReviewValidationError(str(exc)) from exc
Expand All @@ -989,6 +990,7 @@ def _commit_coordinator_inbox(
"verdict": verdict,
"reasoning": item.get("reasoning"),
"source": item.get("source", "critic"),
"failure_reason_code": str(item.get("failure_reason_code") or ""),
"kb_evidence": item.get("kb_evidence") or [],
},
)
Expand Down
5 changes: 5 additions & 0 deletions src/hyperloom/agents/critic/runtime/intent_envelope.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ def build_review_verdict_intent(
alternative_action: str | None = None,
advice_text: str = "",
notes: Iterable[str] | None = None,
failure_reason_code: str = "",
) -> Intent:
"""Build a validated ``review_verdict`` intent.

Expand All @@ -183,6 +184,9 @@ def build_review_verdict_intent(
alternative_action (str | None): Suggested alternative action.
advice_text (str): Devil's-advocate advice text.
notes (Iterable[str] | None): Additional free-text notes.
failure_reason_code (str): The ``failure_reason_code`` of the review
rule this verdict rests on, as declared in the judge bundle's
``review_constraints``. Empty when the verdict cites no rule.

Returns:
Intent: The constructed ``review_verdict`` intent.
Expand Down Expand Up @@ -210,6 +214,7 @@ def build_review_verdict_intent(
"alternative_action": alternative_action,
"advice_text": advice_text,
"notes": list(notes or []),
"failure_reason_code": failure_reason_code,
}
if confidence is not None:
payload["confidence"] = confidence
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,45 @@ def _verdict_intent_for(intents: list[dict], target: str) -> dict:
raise AssertionError(f"no review_verdict intent for {target!r}")


def test_commit_review_carries_the_cited_rule_into_the_intent(reviewer):
"""The Coordinator holds a reject to the verdict its rule declared, and it
can only do that if the code the Critic cited survives the commit path."""
rev, _kb, sm = reviewer
rev.prepare_review(_coordinator_request(_PROMPT_WITH_TWO_PROPOSALS, "sess_code"))
review = {
"review_verdicts": [
{
"target_proposal_msg_id": "aaa1",
"verdict": "reject",
"reasoning": "proposal carried a self-reported gain",
"failure_reason_code": "specialist_quantitative_claim_violation",
},
{
"target_proposal_msg_id": "bbb2",
"verdict": "approve",
"reasoning": "evidence is complete",
},
]
}
outcome = rev.commit_review(
_coordinator_request(_PROMPT_WITH_TWO_PROPOSALS, "sess_code"),
review,
)
intents = outcome.intent_envelope["intents"]

cited = _verdict_intent_for(intents, "aaa1")["payload"]
assert cited["failure_reason_code"] == "specialist_quantitative_claim_violation"
uncited = _verdict_intent_for(intents, "bbb2")["payload"]
assert uncited["failure_reason_code"] == ""
logged = [
json.loads(line)["decision_review"]
for line in (sm.session_dir("sess_code") / "decisions.jsonl").read_text("utf-8").splitlines()
if line.strip()
]
codes = {d["target_proposal_msg_id"]: d["failure_reason_code"] for d in logged}
assert codes["aaa1"] == "specialist_quantitative_claim_violation"


def test_commit_review_backfills_advice_text_from_advice_entry(reviewer):
rev, kb, sm = reviewer
rev.prepare_review(_coordinator_request(_PROMPT_WITH_TWO_PROPOSALS, "sess_advice"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@
CRITIC_AGENT_LLM_CONNECT_TIMEOUT_SEC,
CRITIC_AGENT_LLM_RW_TIMEOUT_SEC,
CRITIC_AGENT_MAX_COMPLETION_TOKENS,
_REVIEW_OUTPUT_INSTRUCTIONS,
_extract_review_json,
_reviewed_msg_ids_from_bundle,
_verdict_references_kb,
)
from hyperloom.orchestrator.specialists.patch_safety import FORBIDDEN_PROPOSAL_FIELDS
from hyperloom.inference_optimizer.protocol.intent import IntentType


Expand Down Expand Up @@ -922,6 +924,91 @@ async def test_user_prompt_includes_judge_bundle_and_instructions(
assert '"abc"' in user_text # proposal msg_id from judge bundle


def test_the_output_schema_asks_for_the_rule_the_verdict_rests_on():
"""The Critic is told to reply with *exactly* this schema, and the
Coordinator holds a reject to the verdict its cited rule declared by reading
`failure_reason_code`. Documenting the field only in a reference file
nothing loads is why prose-scanning became the only signal in production."""
schema, _, rules = _REVIEW_OUTPUT_INSTRUCTIONS.partition("Rules (mirror")

assert '"failure_reason_code"' in schema
assert "failure_reason_code" in rules


def _bundle_reviewing(action_name: str) -> dict[str, Any]:
"""A judge bundle whose single proposal proposes ``action_name``."""
return {
"kind": "coordinator_inbox",
"merged_context": {"model": "m", "framework": "sglang"},
"proposals": [
{
"msg_id": "abc",
"from_agent": "orchestration",
"action_name": action_name,
"payload": {},
"predicted_gain_pct": 0.0,
}
],
"kb_priors_by_proposal": {"abc": []},
"kb_read_skipped_reason": None,
"review_constraints": {},
"notes": [],
"missing_context": [],
"required_context": [],
}


async def _review_constraints_sent_for(
action_name: str,
fake_critic_root: Path,
fake_session_dir: Path,
) -> dict[str, Any]:
"""Run one review turn and return the ``review_constraints`` the model saw."""
backend, client = _make_backend(
fake_critic_root,
fake_session_dir,
codex_replies=['{"review_verdicts": [{"target_proposal_msg_id": "abc", "verdict": "approve"}]}'],
judge_bundle=_bundle_reviewing(action_name),
)
await backend.run("ignored", system_prompt="you are critic")
user_text = client.completions.calls[0]["messages"][1]["content"]
match = re.search(
r"==== JUDGE BUNDLE ====\s*(\{.*?\})\s*==== END JUDGE BUNDLE ====",
user_text,
re.DOTALL,
)
assert match
return json.loads(match.group(1))["review_constraints"]


@pytest.mark.asyncio
async def test_the_reviewed_bundle_carries_the_quantitative_claim_rule(
fake_critic_root: Path,
fake_session_dir: Path,
):
"""Delivered as data so the Critic's field list stays identical to the one
the runner strips, and so a format slip is advisory rather than a reject
that costs the round every proposal in the set."""
constraints = await _review_constraints_sent_for("specialist", fake_critic_root, fake_session_dir)

rule = constraints["quantitative_claim_rule"]
assert rule["failure_verdict"] == "advise"
assert set(rule["forbidden_proposal_fields"]) == set(FORBIDDEN_PROPOSAL_FIELDS)


@pytest.mark.asyncio
async def test_a_review_the_rule_cannot_apply_to_is_not_handed_the_rule(
fake_critic_root: Path,
fake_session_dir: Path,
):
"""The rule is about ``proposal_set[*]``, which a ``baseline`` proposal has
no room for. Sending it anyway invites a citation the verdict path then has
to read, so it goes only where it can be violated."""
constraints = await _review_constraints_sent_for("baseline", fake_critic_root, fake_session_dir)

assert "quantitative_claim_rule" not in constraints


# Static context propagation — backend sources model/framework from manifest.json or explicit static_context.
def _write_manifest(session_dir: Path, payload: dict[str, Any]) -> Path:
"""Write a minimal manifest.json the backend can ingest."""
Expand Down
Loading
Loading