From bed8d60833c6035dee48ff00fd1692daf4d19ad6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 00:31:56 +0000 Subject: [PATCH] feature: honour the prompt header keys the ranker is documented to act on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PyAutoMind REFERENCE.md states the contract: Intake persists Difficulty: from the shared sizing faculty "so the value shown up front is the one the Feature Agent later acts on". It was not acted on. _feature.py mentioned Difficulty once, where it PRINTED its own re-derived score, and read no header key at all — not Difficulty:, not Status:, not Priority:, not Blocked-by:/Closes-when:. The visible failure: `feature select` recommended a prompt declaring Status: blocked, Priority: low AND an explicit Blocked-by: gate whose body says "do not start before the queue runner has run cleanly". Parsing lands in the sizing faculty, beside the derivation it overrides, so declared and derived stay in one place and the bug/refactor conductors get the same reading for free: - declared_header() reads Difficulty/Status/Priority/Blocked-by/Closes-when, skipping fenced blocks. That skip is load-bearing, not tidiness: the bug prompt driving this fix QUOTES the offending header in a ```-block, so without it the bug report would declare itself blocked. Same rule and same reason as PyAutoMind lifecycle.py:draft_gate_refs. - trailing `# note` comments are stripped, splitting on " #" so a Repo#123 ref survives intact (the live backlog has both forms on one line). - an unrecognised Difficulty: value is ignored rather than trusted. The ranker then honours them: declared difficulty overrides derived (with the disagreement REPORTED, not silently resolved — the gap is evidence about the heuristic); Priority: orders the shortlist above the difficulty term; and a prompt declaring Status: blocked or an unresolved Blocked-by: sinks below everything and can never be the recommended pick. It stays listed in its own labelled band so a human can see and override, and next_action says do not start. declared_blocked() is deliberately conservative: this agent is offline and cannot tell whether a gate has since closed, so an unresolved Blocked-by: reads as blocked and the output points at `lifecycle.py issues --drafts`, which does resolve refs against GitHub. Being wrongly held back is cheap and visible; being wrongly recommended is the failure this exists to stop. Measured on the live backlog: the blocked prompt drops from rank 1 to last of 27, correctly labelled, and two further blocked prompts surface that had not been noticed by hand. draft_staleness_detection_signals moves from too-large #26 to medium #5 — it derived too-large purely on file length, and length is a bad size proxy because a prompt grows as it accumulates findings. New tests are hermetic and pin the regression by the offending prompt's HEADER SHAPE rather than the live file, so fixing the backlog cannot silently retire them. Verified they actually bite: reverting only _feature.py fails 7 of the 10. Fixtures use invented repo names, so the file adds no tenant-firewall instance facts. Prompt: PyAutoMind draft/bug/pyautobrain/feature_ranker_ignores_header_keys.md Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WSCA1h6cSKMNwFmWEsxuCL --- agents/conductors/feature/AGENTS.md | 28 +++- agents/conductors/feature/_feature.py | 84 ++++++++++-- agents/faculties/sizing/_sizing.py | 85 ++++++++++++ tests/test_feature_ranker_headers.py | 178 ++++++++++++++++++++++++++ 4 files changed, 362 insertions(+), 13 deletions(-) create mode 100644 tests/test_feature_ranker_headers.py diff --git a/agents/conductors/feature/AGENTS.md b/agents/conductors/feature/AGENTS.md index 9159fb7..01d63fa 100644 --- a/agents/conductors/feature/AGENTS.md +++ b/agents/conductors/feature/AGENTS.md @@ -44,12 +44,36 @@ it reasons over and the PyAutoMemory routing it uses. | **selection** | no task given | Scan `draft/feature/**` (legacy flat `feature/**` still resolves), rank candidates, and recommend the best next task — **not** merely the first in a list; down-ranks in-flight work (from `active.md` / `planned.md`). | | **difficulty-constrained** | `--difficulty` / `--model` / `--budget` / `--ambitious` / `--impact` | Estimate difficulty per task and select to match the constraint (easy/weak-model/limited-token → small; ambitious/strong-model → large; impact → high-leverage). | +## The declared header wins + +A prompt's metadata header (PyAutoMind `REFERENCE.md`, "Optional metadata +header") is **read, not decoration**: + +| Key | Effect on the ranker | +|---|---| +| `Difficulty:` | Overrides the derived level. Intake persists it from this same sizing faculty, so it *is* the value this agent acts on; a declared/derived disagreement is reported, never silently resolved. | +| `Priority:` | Orders the shortlist (`high` → `normal` → `low`), above the difficulty term. | +| `Status: blocked` | Sinks the prompt below everything and bars it from being the recommended pick. | +| `Blocked-by:` | Same, on its own — an unresolved gate reads as blocked. | + +Blocked prompts stay **listed, in their own band**, so a human can see and +override; they are never recommended. Gate *state* is not resolved here — this +agent is offline. `PyAutoMind/scripts/lifecycle.py issues --drafts` checks the +refs against GitHub and is the tool that says a `Blocked-by:` has cleared. + +Keys inside fenced code blocks are documentation and are ignored, so a prompt may +quote another's header without inheriting it. + ## Difficulty & sizing -Difficulty is a transparent heuristic (`small | medium | large | too-large`) over +When nothing is declared, difficulty is a transparent heuristic +(`small | medium | large | too-large`) over repos affected, prompt size, scientific complexity, architectural risk, test burden, and whether memory context / human judgement is required. The factor -breakdown is in every decision so the reasoning layer can adjust. +breakdown is in every decision so the reasoning layer can adjust. Note the +prompt-size term: a prompt grows as it accumulates findings, so a long, +well-documented prompt can derive `too-large` for work that is not — which is +exactly what a declared `Difficulty:` is for. Sizing then drives the **phase decision**: diff --git a/agents/conductors/feature/_feature.py b/agents/conductors/feature/_feature.py index 189f57d..0d67195 100755 --- a/agents/conductors/feature/_feature.py +++ b/agents/conductors/feature/_feature.py @@ -36,6 +36,7 @@ policy as _sizing_policy, TEST_KEYWORDS, normalise_repo, parse_prompt, discover_prompts, empty_discovery_reason, estimate_difficulty, _hits, _within, + declared_blocked, priority_rank, ) # Default sub-wiki to consult per library target when no keyword fires. Memory @@ -158,8 +159,22 @@ def risks(level: str, factors: dict, workflow: str): return out +def effective_difficulty(p: dict): + """(level, score, factors, derived_level) — the DECLARED level wins. + + REFERENCE.md promises that the `Difficulty:` Intake persists is "the value + the Feature Agent later acts on", so a declared level overrides the + re-derived one. The derived score is kept: it still orders prompts within a + level, and the derived LEVEL is returned alongside so a disagreement can be + reported rather than silently resolved — the disagreement is evidence about + the heuristic and is worth seeing. + """ + derived_level, score, factors = estimate_difficulty(p) + return p.get("declared_difficulty") or derived_level, score, factors, derived_level + + def analyse(p: dict): - level, score, factors = estimate_difficulty(p) + level, score, factors, derived_level = effective_difficulty(p) workflow, rehome = recommend_workflow(p, factors) mem = memory_context(p) phase, stubs = phase_decision(level, factors, p) @@ -169,6 +184,14 @@ def analyse(p: dict): "target": p["target"], "repos_affected": p["repos"], "difficulty": level, + "difficulty_declared": p.get("declared_difficulty"), + "difficulty_derived": derived_level, + "difficulty_disagreement": ( + p.get("declared_difficulty") is not None and derived_level != level + ), + "priority": p.get("priority"), + "status": p.get("status"), + "blocked": declared_blocked(p), "difficulty_score": score, "difficulty_factors": factors, "recommended_workflow": workflow, @@ -207,13 +230,18 @@ def select(mind: Path, constraint: dict, limit: int): rows = [] for path in prompts: p = parse_prompt(path, mind) - level, score, factors = estimate_difficulty(p) + level, score, factors, derived_level = effective_difficulty(p) impact = score + (2 if factors["library_and_workspace"] else 0) \ + len(factors["scientific_complexity"]) rows.append({ "path": p["path"], "difficulty": level, "score": score, "impact": impact, "repos": p["repos"], "in_flight": p["path"] in in_flight, + "blocked": declared_blocked(p), + "priority": p.get("priority"), + "priority_rank": priority_rank(p), + "difficulty_declared": p.get("declared_difficulty"), + "difficulty_derived": derived_level, "factors": factors, }) @@ -225,15 +253,19 @@ def select(mind: Path, constraint: dict, limit: int): impact_pref = constraint.get("impact") def keyfn(r): - # Down-rank in-flight work so we never just resurface active tasks. - penalty = 100 if r["in_flight"] else 0 + # A prompt that declares itself blocked sinks below everything, so it can + # never be the recommended pick — it stays listed, in its own band, so a + # human can still see it and override. + # Then: in-flight work is down-ranked so we never just resurface active + # tasks; then declared Priority:, which is an ordering input and not + # merely display; then the constraint's own difficulty term. + head = (200 if r["blocked"] else 0) + (100 if r["in_flight"] else 0) + prio = r["priority_rank"] if impact_pref: - return (penalty, -r["impact"]) + return (head, prio, -r["impact"]) if model == "strong" or constraint.get("ambitious"): - return (penalty, -r["score"]) - if model == "weak" or budget or want in ("easy", "small"): - return (penalty, r["score"]) - return (penalty, r["score"]) # default: easiest-first, stable + return (head, prio, -r["score"]) + return (head, prio, r["score"]) # default: easiest-first, stable candidates = rows if want and want not in ("easy",): @@ -255,7 +287,19 @@ def emit_human(mode: str, decision: dict): print(f"Mode: {mode}") print(f"Work-type / target: {d['work_type']} / {d['target']}") print(f"Repos affected: {', '.join(d['repos_affected']) or '(none resolved)'}") - print(f"Difficulty: {d['difficulty']} (score {d['difficulty_score']})") + src = "declared" if d.get("difficulty_declared") else "derived" + print(f"Difficulty: {d['difficulty']} ({src}, score {d['difficulty_score']})") + if d.get("difficulty_disagreement"): + # Surfaced, not silently resolved: the declared value governs, but the + # gap is evidence about the sizing heuristic and someone should see it. + print(f" ! declared {d['difficulty_declared']} but derived " + f"{d['difficulty_derived']} — declared wins; disagreement worth a look") + if d.get("priority"): + print(f"Priority: {d['priority']} (declared)") + if d.get("blocked"): + print(f"BLOCKED (declared): {d['blocked']}") + print(" gate state is NOT resolved here — " + "`lifecycle.py issues --drafts` checks it against GitHub") print(f"Recommended workflow: {d['recommended_workflow']}", end="") print(f" [re-home as {d['rehome_suggestion']}/]" if d["rehome_suggestion"] else "") if d["memory_context"]: @@ -279,6 +323,10 @@ def emit_human(mode: str, decision: dict): def _next_action(d: dict): + if d.get("blocked"): + return (f"Do NOT start — the prompt declares {d['blocked']}. Clear the gate " + f"(or correct the header) first; `lifecycle.py issues --drafts` " + f"resolves gate state against GitHub.") if d["rehome_suggestion"]: return f"Re-home this prompt under {d['rehome_suggestion']}/ and scope it before development." if d["phase_decision"] == "split-into-phases": @@ -347,10 +395,24 @@ def main(argv=None): print(f"== Feature task {mode} ({total} feature prompts considered) ==") print("Shortlist (recommendation — apply priorities/dependencies/health on top):") for i, r in enumerate(ranked): - flag = " [in-flight, down-ranked]" if r["in_flight"] else "" + flags = [] + if r["blocked"]: + flags.append(f"BLOCKED — {r['blocked']}") + if r["in_flight"]: + flags.append("in-flight, down-ranked") + if r.get("priority") and r["priority"] != "normal": + flags.append(f"priority {r['priority']}") + if r.get("difficulty_declared") and r["difficulty_derived"] != r["difficulty"]: + flags.append(f"declared {r['difficulty_declared']} vs derived " + f"{r['difficulty_derived']}") + flag = f" [{'; '.join(flags)}]" if flags else "" print(f" {i+1}. {r['path']} [{r['difficulty']}, score {r['score']}, " f"impact {r['impact']}]{flag}") print() + if ranked[0]["blocked"]: + # Every candidate is blocked — say so instead of recommending one anyway. + print("NOTE: every shortlisted prompt declares itself blocked; the pick " + "below is shown for context and should NOT be started as-is.\n") chosen = parse_prompt(mind / ranked[0]["path"], mind) decision = analyse(chosen) print("Recommended pick (not merely the first prompt — ranked by the constraint):") diff --git a/agents/faculties/sizing/_sizing.py b/agents/faculties/sizing/_sizing.py index aa3aece..fea4b0c 100755 --- a/agents/faculties/sizing/_sizing.py +++ b/agents/faculties/sizing/_sizing.py @@ -224,6 +224,90 @@ def empty_discovery_reason(mind: Path, work_type: str) -> str: return f"{where} exists under {mind} but holds no prompts (backlog genuinely empty)" +# --- the declared metadata header ------------------------------------------- +# +# PyAutoMind/REFERENCE.md ("Optional metadata header") defines these keys and +# states the contract this parser exists to honour: Intake persists `Difficulty:` +# "so the value shown up front is the one the Feature Agent later acts on". +# Parsing them here — beside the derivation — keeps declared and derived in one +# place, and gives the bug/refactor conductors the same reading for free. +DIFFICULTY_LEVELS = ("small", "medium", "large", "too-large") +# `medium` is not a documented Priority: value but occurs in the live backlog; +# read it as normal rather than dropping the prompt's stated intent. +PRIORITY_RANK = {"high": 0, "normal": 1, "medium": 1, "low": 2} +DEFAULT_PRIORITY_RANK = 1 + +_HEADER_KEY_RE = re.compile( + r"^\s*(difficulty|status|priority|blocked-by|closes-when)\s*:\s*(.+?)\s*$", re.I +) + + +def _strip_trailing_comment(value: str) -> str: + """Header values may carry a trailing `# note` (the live backlog does, e.g. + `Blocked-by: PyAutoFit#1334 # WP1 gate (MERGED)`). Split on ` #` so a + `Repo#123` ref — which has no space before the hash — survives intact.""" + return value.split(" #", 1)[0].strip() + + +def declared_header(text: str) -> dict: + """The header keys a prompt *declares*, as opposed to what we infer. + + Fenced blocks are documentation, not declarations — a prompt that quotes + another prompt's header in a ```-block (the bug prompt for this very fix + does exactly that) must not be read as declaring it. Same rule, and the + same reason, as PyAutoMind `lifecycle.py:draft_gate_refs`. + """ + out = {"declared_difficulty": None, "status": None, + "priority": None, "blocked_by": [], "closes_when": []} + in_fence = False + for line in text.splitlines(): + if line.lstrip().startswith("```"): + in_fence = not in_fence + continue + if in_fence: + continue + m = _HEADER_KEY_RE.match(line) + if not m: + continue + key, value = m.group(1).lower(), _strip_trailing_comment(m.group(2)) + if not value: + continue + if key == "difficulty": + v = value.lower() + if v in DIFFICULTY_LEVELS and out["declared_difficulty"] is None: + out["declared_difficulty"] = v + elif key == "status" and out["status"] is None: + out["status"] = value.lower() + elif key == "priority" and out["priority"] is None: + out["priority"] = value.lower() + elif key == "blocked-by": + out["blocked_by"].append(value) + elif key == "closes-when": + out["closes_when"].append(value) + return out + + +def priority_rank(p: dict) -> int: + return PRIORITY_RANK.get(p.get("priority") or "", DEFAULT_PRIORITY_RANK) + + +def declared_blocked(p: dict): + """Why the prompt declares itself un-startable, or None. + + Deliberately conservative: this faculty is offline, so it cannot resolve + whether a `Blocked-by:` gate has since closed — that is + `PyAutoMind/scripts/lifecycle.py issues --drafts`, which talks to GitHub. An + unresolved gate therefore reads as blocked. Being wrongly held back is cheap + and visible (the prompt is still listed, in its own band); being wrongly + recommended is the failure this exists to stop. + """ + if (p.get("status") or "") == "blocked": + return "Status: blocked" + if p.get("blocked_by"): + return "Blocked-by: " + "; ".join(p["blocked_by"]) + return None + + def parse_prompt(path: Path, mind: Path): """Read a prompt file and extract structure: work-type, target, repos, body.""" text = path.read_text(encoding="utf-8", errors="replace") @@ -282,6 +366,7 @@ def parse_prompt(path: Path, mind: Path): "text": text, "lines": text.count("\n") + 1, "words": len(text.split()), + **declared_header(text), } diff --git a/tests/test_feature_ranker_headers.py b/tests/test_feature_ranker_headers.py new file mode 100644 index 0000000..5e0689b --- /dev/null +++ b/tests/test_feature_ranker_headers.py @@ -0,0 +1,178 @@ +"""tests/test_feature_ranker_headers.py — the ranker honours declared headers. + +PyAutoMind/REFERENCE.md promises that the `Difficulty:` Intake persists is "the +value the Feature Agent later acts on", and defines `Status:` / `Priority:` / +`Blocked-by:` alongside it. The ranker previously read none of them, so a prompt +declaring `Status: blocked`, `Priority: low` AND an explicit `Blocked-by:` gate +came back as the recommended next pick. + +Hermetic: every test fabricates a temp Mind, so nothing depends on the live +backlog — including the regression case, which pins the offending prompt's +HEADER SHAPE rather than the file itself. Fixtures use invented repo names so +the file carries no instance facts. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path + +BRAIN_HOME = Path(__file__).resolve().parents[1] +FEATURE = BRAIN_HOME / "agents" / "conductors" / "feature" / "_feature.py" +sys.path.insert(0, str(BRAIN_HOME / "agents" / "faculties" / "sizing")) + +from _sizing import declared_blocked, declared_header, parse_prompt # noqa: E402 + + +def _write(mind: Path, rel: str, body: str) -> Path: + p = mind / rel + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(body) + return p + + +def _prompt(title, *, difficulty=None, priority=None, status=None, + blocked_by=None, extra=""): + head = [f"# {title}", "", "Type: feature", "Target: widgets"] + if difficulty: + head.append(f"Difficulty: {difficulty}") + if priority: + head.append(f"Priority: {priority}") + if status: + head.append(f"Status: {status}") + if blocked_by: + head.append(f"Blocked-by: {blocked_by}") + return "\n".join(head) + "\n\n" + (extra or "Some body text.") + "\n" + + +def _select(mind: Path, *args): + r = subprocess.run( + [sys.executable, str(FEATURE), "--mind", str(mind), "--json", + "select", "--limit", "20", *args], + capture_output=True, text=True, + ) + assert r.returncode == 0, r.stderr + return json.loads(r.stdout) + + +def _order(shortlist): + return [row["path"] for row in shortlist] + + +# --- the regression case ------------------------------------------------------ + +# The header shape of draft/feature/autonomy/10_scheduled_runs.md, which the +# ranker recommended as the next task to start. Pinned as a fixture, not read +# from the live file, so fixing the backlog cannot silently retire this test. +BLOCKED_SHAPE = _prompt( + "Scheduled runs", + difficulty="medium", priority="low", status="blocked", + blocked_by="7_queue_runner.md (and transitively 1-5)", + extra="Do not start before the queue runner has run cleanly.", +) + + +def test_blocked_prompt_is_never_the_recommended_pick(tmp_path): + _write(tmp_path, "draft/feature/autonomy/scheduled_runs.md", BLOCKED_SHAPE) + _write(tmp_path, "draft/feature/widgets/ordinary.md", _prompt("Ordinary")) + d = _select(tmp_path) + assert d["selected_task"].endswith("ordinary.md") + # still listed, so a human can see and override it — just never recommended + assert _order(d["shortlist"])[-1].endswith("scheduled_runs.md") + assert d["shortlist"][-1]["blocked"] + + +def test_blocked_by_alone_is_enough_to_sink_a_prompt(tmp_path): + _write(tmp_path, "draft/feature/widgets/gated.md", + _prompt("Gated", blocked_by="Widget#12, Gadget#7")) + _write(tmp_path, "draft/feature/widgets/ordinary.md", _prompt("Ordinary")) + d = _select(tmp_path) + assert d["selected_task"].endswith("ordinary.md") + assert _order(d["shortlist"])[-1].endswith("gated.md") + + +def test_next_action_refuses_to_start_a_blocked_pick(tmp_path): + _write(tmp_path, "draft/feature/widgets/only_one.md", BLOCKED_SHAPE) + d = _select(tmp_path) + assert "Do NOT start" in d["next_action"] + + +# --- declared difficulty wins ------------------------------------------------- + +# A short prompt derives `small`; declaring `too-large` must override it. +def test_declared_difficulty_overrides_the_derived_level(tmp_path): + p = _write(tmp_path, "draft/feature/widgets/declared.md", + _prompt("Declared", difficulty="too-large")) + d = _select(tmp_path) + row = d["shortlist"][0] + assert row["difficulty"] == "too-large" + assert row["difficulty_declared"] == "too-large" + assert row["difficulty_derived"] != "too-large" + assert d["difficulty_disagreement"] is True + assert parse_prompt(p, tmp_path)["declared_difficulty"] == "too-large" + + +def test_derived_difficulty_still_used_when_none_declared(tmp_path): + _write(tmp_path, "draft/feature/widgets/plain.md", _prompt("Plain")) + d = _select(tmp_path) + row = d["shortlist"][0] + assert row["difficulty_declared"] is None + assert row["difficulty"] == row["difficulty_derived"] + assert d["difficulty_disagreement"] is False + + +# --- priority orders the shortlist -------------------------------------------- + +def test_priority_orders_two_otherwise_equal_prompts(tmp_path): + body = "Some body text." + _write(tmp_path, "draft/feature/widgets/a_low.md", + _prompt("A", priority="low", extra=body)) + _write(tmp_path, "draft/feature/widgets/b_high.md", + _prompt("B", priority="high", extra=body)) + order = _order(_select(tmp_path)["shortlist"]) + assert order[0].endswith("b_high.md") + assert order[1].endswith("a_low.md") + + +def test_priority_is_honoured_under_the_impact_constraint_too(tmp_path): + body = "Some body text." + _write(tmp_path, "draft/feature/widgets/a_low.md", + _prompt("A", priority="low", extra=body)) + _write(tmp_path, "draft/feature/widgets/b_high.md", + _prompt("B", priority="high", extra=body)) + order = _order(_select(tmp_path, "--impact")["shortlist"]) + assert order[0].endswith("b_high.md") + + +# --- fenced blocks are documentation, not declarations ------------------------ + +def test_keys_inside_a_fence_are_not_read_as_declarations(): + """REFERENCE.md's own rule, and the reason it exists: a prompt that QUOTES + another prompt's header (the bug prompt for this fix does) must not inherit + it. Without this, the bug report would declare itself blocked.""" + text = ( + "# Explaining the keys\n\nType: bug\nPriority: high\n\n" + "```\nDifficulty: too-large\nStatus: blocked\nPriority: low\n" + "Blocked-by: Widget#1\n```\n\nreal body\n" + ) + h = declared_header(text) + assert h["priority"] == "high" # the real declaration, outside the fence + assert h["declared_difficulty"] is None # the quoted ones are documentation + assert h["status"] is None + assert h["blocked_by"] == [] + assert declared_blocked(h) is None + + +def test_trailing_comment_is_stripped_but_a_ref_hash_survives(): + h = declared_header( + "Difficulty: medium # small | medium | large\n" + "Blocked-by: Widget#1334, Gadget#1331 # WP1 gate (MERGED)\n" + ) + assert h["declared_difficulty"] == "medium" + assert h["blocked_by"] == ["Widget#1334, Gadget#1331"] + + +def test_unknown_difficulty_value_is_ignored_rather_than_trusted(): + assert declared_header("Difficulty: enormous\n")["declared_difficulty"] is None