mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
* feat(gateway): delegation detail-fidelity — details survive hand-off, both directions
Details thinned out at every delegation hop: a PM child task mapped to no
parent criterion was legal (coverage only surfaced at submit_up, after the
whole wave ran — a 12-subtask docs tree grew through 8 review rounds that
way, one child titled 'docs page and route wrapper' shipping only the
page), and QA could pass work on a gestalt read (a 4-scene video brief
shipped 3 scenes past every gate because the features existed only in
prose). Three chokepoint gates:
- delegate (down): every child must declare covers_parent_criteria
resolving against the parent's real acceptance criteria — no mapping or
an unresolvable ref rejects naming every offending child and the valid
criteria; the success envelope carries parent_ac_coverage
{covered, uncovered} so a wave-planning PM sees remaining gaps in the
same turn. Full coverage stays enforced at submit_up (waves stay legal).
- pass_review (up): mandatory criteria_verified — one {criterion,
evidence} entry per task AC, matched by the findings ledger's
id-or-exact-text matcher, evidence soup-checked and capped; rejects
naming the unverified criteria; entries render deterministically into
qa_notes as '[AC] <criterion> — verified: <evidence>' lines. The old
count-only ac_verdicts gate is superseded (arg kept for back-compat).
- video briefs (structured detail at origination): an enumerable feature
list (release highlights, or input_props.highlights carried onto a
reject re-author) becomes its own scene acceptance criterion, bounded to
the AC caps; a re-author without highlights carries the
feedback-addressed criterion instead.
Extracted findings.py's criterion matcher into shared unmatched_criteria /
uncovered_acceptance_criteria instead of duplicating it; criteria_verified
joins the WAF free-text exclusion set like findings/issues.
* fix(gateway): break the block/unblock wedge — four hardening fixes from the live PM loop
A cell task looped fe-pm/main-pm block/unblock for hours (10 cycles, 43
spawns): a transient GitHub API error resolving CI became an unwaivable
blocker finding whose own fix text said no code change was required, the
submit freshness guard then demanded a commit no finding called for,
escalate_up auto-blocked, and main-pm's correct recovery plan 422'd on
the approach length cap, degrading it to a bare unblock. Four fixes:
- pr_pass CI-unresolvable refusal is now explicitly transient-worded:
retry pr_pass shortly, do NOT pr_fail over a CI-status lookup error —
a platform blip is not a code finding
- submit freshness guard grants ONE unchanged-head resubmission per
head sha when the findings ledger has zero open rows (all addressed
without code changes) — stamped via the resubmit_unchanged_head
marker so the same head can never loop a second time
- unblock carries a flip breaker: block_flip_count marker, and at the
third flip a one-shot CEO notification flags the task as structurally
wedged (unblock itself still succeeds — the breaker signals, it does
not wedge recovery)
- i_will_plan's approach cap truncates at 800 chars instead of
rejecting — an over-detailed plan must never cost the PM its turn
---------
Co-authored-by: Renn F <rennf93@users.noreply.github.com>
176 lines
5.4 KiB
Python
176 lines
5.4 KiB
Python
"""pass_review requires a matched, evidenced verification per acceptance
|
|
criterion — not just a count of arbitrary strings.
|
|
|
|
Live failure this closes: QA passed a rendered video shipping 3 of the
|
|
brief's 4 named scenes because nothing forced the reviewer to walk each
|
|
acceptance criterion individually. Mirrors the test idiom in
|
|
test_qa_ac_coverage.py, one level stricter: criteria_verified entries must
|
|
each match a real AC (by id or exact text) and carry substantive evidence.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from types import SimpleNamespace
|
|
|
|
from roboco.services.gateway.choreographer import Choreographer
|
|
|
|
_EVIDENCE_CAP = 500
|
|
|
|
|
|
def _task(criteria: list[str], ids: list[str] | None = None) -> SimpleNamespace:
|
|
return SimpleNamespace(
|
|
acceptance_criteria=criteria,
|
|
acceptance_criteria_ids=ids or [],
|
|
)
|
|
|
|
|
|
def test_no_criteria_imposes_no_requirement() -> None:
|
|
pairs, rej = Choreographer._validate_criteria_verified(_task([]), None)
|
|
assert pairs == []
|
|
assert rej is None
|
|
|
|
|
|
def test_none_supplied_lists_every_criterion_verbatim() -> None:
|
|
criteria = [
|
|
"scene 1 renders",
|
|
"scene 2 renders",
|
|
"scene 3 renders",
|
|
"scene 4 renders",
|
|
]
|
|
t = _task(criteria)
|
|
pairs, rej = Choreographer._validate_criteria_verified(t, None)
|
|
assert pairs == []
|
|
assert rej is not None
|
|
body = rej.as_dict()
|
|
assert body["error"] == "invalid_state", body
|
|
for crit in criteria:
|
|
assert crit in body["message"]
|
|
|
|
|
|
def test_empty_list_is_treated_as_none_supplied() -> None:
|
|
t = _task(["a"])
|
|
pairs, rej = Choreographer._validate_criteria_verified(t, [])
|
|
assert pairs == []
|
|
assert rej is not None
|
|
|
|
|
|
def test_partial_coverage_names_the_missing_criterion() -> None:
|
|
t = _task(["a", "b", "c"])
|
|
pairs, rej = Choreographer._validate_criteria_verified(
|
|
t,
|
|
[
|
|
{"criterion": "a", "evidence": "frame 1 shows a rendered"},
|
|
{"criterion": "b", "evidence": "frame 2 shows b rendered"},
|
|
],
|
|
)
|
|
assert pairs == []
|
|
assert rej is not None
|
|
assert "c" in rej.as_dict()["message"]
|
|
|
|
|
|
def test_unmatched_criterion_is_rejected_naming_valid_ones() -> None:
|
|
t = _task(["a", "b"])
|
|
pairs, rej = Choreographer._validate_criteria_verified(
|
|
t,
|
|
[
|
|
{"criterion": "a", "evidence": "frame 1 shows a rendered"},
|
|
{"criterion": "not-a-real-ac", "evidence": "frame 2 shows something"},
|
|
],
|
|
)
|
|
assert pairs == []
|
|
assert rej is not None
|
|
body = rej.as_dict()
|
|
assert "not-a-real-ac" in body["message"]
|
|
assert "a" in body["remediate"] and "b" in body["remediate"]
|
|
|
|
|
|
def test_missing_criterion_key_is_rejected() -> None:
|
|
t = _task(["a"])
|
|
pairs, rej = Choreographer._validate_criteria_verified(
|
|
t, [{"evidence": "frame 1 shows a rendered"}]
|
|
)
|
|
assert pairs == []
|
|
assert rej is not None
|
|
|
|
|
|
def test_blank_evidence_is_rejected() -> None:
|
|
t = _task(["a"])
|
|
pairs, rej = Choreographer._validate_criteria_verified(
|
|
t, [{"criterion": "a", "evidence": " "}]
|
|
)
|
|
assert pairs == []
|
|
assert rej is not None
|
|
|
|
|
|
def test_soup_evidence_is_rejected() -> None:
|
|
t = _task(["a"])
|
|
pairs, rej = Choreographer._validate_criteria_verified(
|
|
t, [{"criterion": "a", "evidence": "wip"}]
|
|
)
|
|
assert pairs == []
|
|
assert rej is not None
|
|
|
|
|
|
def test_overlong_evidence_is_rejected() -> None:
|
|
t = _task(["a"])
|
|
pairs, rej = Choreographer._validate_criteria_verified(
|
|
t, [{"criterion": "a", "evidence": "x" * (_EVIDENCE_CAP + 100)}]
|
|
)
|
|
assert pairs == []
|
|
assert rej is not None
|
|
assert str(_EVIDENCE_CAP) in rej.as_dict()["message"]
|
|
|
|
|
|
def test_full_coverage_by_exact_text_passes() -> None:
|
|
t = _task(["a", "b"])
|
|
pairs, rej = Choreographer._validate_criteria_verified(
|
|
t,
|
|
[
|
|
{"criterion": "a", "evidence": "frame 1 shows a rendered fully"},
|
|
{"criterion": "b", "evidence": "frame 2 shows b rendered fully"},
|
|
],
|
|
)
|
|
assert rej is None
|
|
assert pairs == [
|
|
("a", "frame 1 shows a rendered fully"),
|
|
("b", "frame 2 shows b rendered fully"),
|
|
]
|
|
|
|
|
|
def test_full_coverage_by_ac_id_passes() -> None:
|
|
t = _task(["scene renders"], ids=["AC-1"])
|
|
pairs, rej = Choreographer._validate_criteria_verified(
|
|
t, [{"criterion": "AC-1", "evidence": "rendered-frame path: out/frame3.png"}]
|
|
)
|
|
assert rej is None
|
|
assert pairs == [("AC-1", "rendered-frame path: out/frame3.png")]
|
|
|
|
|
|
def test_extra_entries_beyond_the_ac_set_are_allowed() -> None:
|
|
t = _task(["a"])
|
|
pairs, rej = Choreographer._validate_criteria_verified(
|
|
t,
|
|
[{"criterion": "a", "evidence": "frame 1 shows a rendered fully"}],
|
|
)
|
|
assert rej is None
|
|
assert len(pairs) == 1
|
|
|
|
|
|
def test_render_criteria_verified_matches_style() -> None:
|
|
lines = Choreographer._render_criteria_verified(
|
|
[("scene 1 renders", "frame 12 shows scene 1 fully")]
|
|
)
|
|
assert lines == ["[AC] scene 1 renders — verified: frame 12 shows scene 1 fully"]
|
|
|
|
|
|
def test_merge_criteria_verified_into_notes() -> None:
|
|
merged = Choreographer._merge_criteria_verified_into_notes(
|
|
"base review", [("a", "evidence a"), ("b", "evidence b")]
|
|
)
|
|
assert "[AC] a — verified: evidence a" in merged
|
|
assert "[AC] b — verified: evidence b" in merged
|
|
|
|
|
|
def test_merge_with_no_pairs_returns_notes_unchanged() -> None:
|
|
assert Choreographer._merge_criteria_verified_into_notes("base", []) == "base"
|