From c10517cf0701aa6fd8dedd842315cdd4b8cddece Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 18 Jul 2026 17:50:48 -0700 Subject: [PATCH 1/9] feat(recipe): add extract_blockquote_sections and extract_blockquote_placeholders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add two new parser helpers to _skill_placeholder_parser.py: - extract_blockquote_sections(content) yields (step_context, block_text) tuples for contiguous blockquote runs of 3+ lines OR any blockquote containing a {*_content} placeholder. Single-line stylistic callouts (e.g., '> **Note:**') are excluded. Tracks the nearest '### Step N' heading as step_context. Flushes trailing blockquotes at EOF. - extract_blockquote_placeholders(text) returns the set of {*_content} variable names found in a blockquote. The *_content naming convention is the wrong-shape signal — subagent prompts must use *_path with the subagent reading the file. These helpers unblock the inline-content-in-subagent-prompt semantic rule (#3636) which currently has no way to detect dangling {*_content} variables in blockquoted subagent prompts. Co-Authored-By: Claude --- .../recipe/_skill_placeholder_parser.py | 69 +++++++++++++ tests/recipe/test_skill_placeholder_parser.py | 99 ++++++++++++++++++- 2 files changed, 167 insertions(+), 1 deletion(-) diff --git a/src/autoskillit/recipe/_skill_placeholder_parser.py b/src/autoskillit/recipe/_skill_placeholder_parser.py index 10d24fffcd..1d6404698a 100644 --- a/src/autoskillit/recipe/_skill_placeholder_parser.py +++ b/src/autoskillit/recipe/_skill_placeholder_parser.py @@ -215,3 +215,72 @@ def extract_validation_rule_block(content: str, rule_label: str) -> str | None: if m.group(1) == rule_label: return m.group(0).strip() return None + + +_CONTENT_VAR_SIGNAL_RE = re.compile(r"\{[a-z_]+_content\}") + + +def extract_blockquote_sections(content: str) -> list[tuple[str, str]]: + """Extract blockquote sections from SKILL.md content that are subagent prompts. + + Returns ``(step_context, block_text)`` tuples where ``step_context`` is the + nearest ``### `` heading above the block and ``block_text`` is the joined + blockquote content with the ``> `` prefix stripped. + + Inclusion criteria — a contiguous blockquote run is yielded if ANY of: + + - It contains at least 3 contiguous ``> `` lines (likely a subagent prompt) + - It contains a ``{*_content}`` placeholder (content signal — even 1-2 line + blocks with these are subagent-prompt-shaped, not stylistic callouts) + + Single-line ``>`` callouts without content signals (e.g., ``> **Note:**``) + are excluded — those are stylistic, not subagent prompts. + + A trailing blockquote that runs to end of file is flushed (not silently + dropped). + """ + sections: list[tuple[str, str]] = [] + current_heading = "" + buf: list[str] = [] + + def flush() -> None: + if not buf: + return + joined = "\n".join(buf) + has_content_signal = bool(_CONTENT_VAR_SIGNAL_RE.search(joined)) + if len(buf) >= 3 or has_content_signal: + sections.append((current_heading, joined)) + buf.clear() + + for line in content.splitlines(): + heading_match = _STEP_RE.match(line) + if heading_match: + flush() + current_heading = heading_match.group(1) + continue + if line.startswith("> "): + buf.append(line[2:]) + elif line.strip() == ">": + buf.append("") + elif line.startswith(">"): + # '>' without space (rare) — still blockquote, strip the '>' + buf.append(line[1:].lstrip()) + else: + flush() + + flush() # trailing blockquote at EOF + return sections + + +_BANNED_CONTENT_SUFFIX_RE = re.compile(r"\{([a-z_]+_content)\}") + + +def extract_blockquote_placeholders(blockquote_text: str) -> set[str]: + """Extract ``{identifier}`` tokens from blockquote text matching ``*_content``. + + The naming convention is the key signal: ``*_content`` in a subagent-facing + blockquote is always wrong — should be ``*_path`` with the subagent reading + the file. Other placeholder patterns (``*_path``, ``*_name``, etc.) are + intentionally excluded; those are valid. + """ + return {m.group(1) for m in _BANNED_CONTENT_SUFFIX_RE.finditer(blockquote_text)} diff --git a/tests/recipe/test_skill_placeholder_parser.py b/tests/recipe/test_skill_placeholder_parser.py index 401856eee0..798166249d 100644 --- a/tests/recipe/test_skill_placeholder_parser.py +++ b/tests/recipe/test_skill_placeholder_parser.py @@ -2,7 +2,11 @@ import pytest -from autoskillit.recipe._skill_placeholder_parser import extract_validation_rule_block +from autoskillit.recipe._skill_placeholder_parser import ( + extract_blockquote_placeholders, + extract_blockquote_sections, + extract_validation_rule_block, +) pytestmark = [pytest.mark.layer("recipe"), pytest.mark.small] @@ -55,3 +59,96 @@ def test_extract_validation_rule_block_different_rule() -> None: assert "V1" not in block assert "V2" in block assert "V3" not in block + + +# --- extract_blockquote_sections --- + + +def test_extract_blockquote_sections_identifies_contiguous_blocks() -> None: + """A 3+ line contiguous blockquote run is yielded with the nearest step heading.""" + sample = ( + "### Step 3: Dispatch subagent\n" + "\n" + "> Review the diff.\n" + "> Report findings.\n" + "> Be concise.\n" + "\n" + "Trailing prose.\n" + ) + blocks = extract_blockquote_sections(sample) + assert len(blocks) == 1 + heading, text = blocks[0] + assert heading == "Step 3" + assert "Review the diff." in text + assert "Report findings." in text + assert "Be concise." in text + + +def test_extract_blockquote_sections_excludes_single_line_callouts() -> None: + """Single-line `>` callouts without content signals are stylistic, not prompts.""" + sample = "### Step 1: Note\n\n> **Note:** This is a stylistic callout.\n\nBody prose.\n" + blocks = extract_blockquote_sections(sample) + assert blocks == [] + + +def test_extract_blockquote_sections_includes_two_line_prompt_with_banned_var() -> None: + """A 2-line blockquote containing a {*_content} placeholder IS returned (content signal).""" + sample = ( + "### Step 2: Inline content\n\n> Review {annotated_diff_content}.\n> Report findings.\n" + ) + blocks = extract_blockquote_sections(sample) + assert len(blocks) == 1 + heading, text = blocks[0] + assert heading == "Step 2" + assert "{annotated_diff_content}" in text + + +def test_extract_blockquote_sections_strips_prefix() -> None: + """Returned text has the `> ` prefix stripped from each line.""" + sample = "### Step 4\n\n> First line.\n> Second line.\n> Third line.\n" + blocks = extract_blockquote_sections(sample) + assert len(blocks) == 1 + _, text = blocks[0] + for line in text.splitlines(): + assert not line.startswith(">"), f"Prefix not stripped: {line!r}" + + +def test_extract_blockquote_sections_trailing_block_at_eof() -> None: + """A blockquote that runs to end of file is flushed, not silently dropped.""" + sample = "### Step 5\n\n> Line one.\n> Line two.\n> Line three." + blocks = extract_blockquote_sections(sample) + assert len(blocks) == 1 + heading, text = blocks[0] + assert heading == "Step 5" + assert "Line one." in text + assert "Line two." in text + assert "Line three." in text + + +def test_extract_blockquote_sections_tuple_order_heading_first() -> None: + """First tuple element is the step heading, second is the body text.""" + sample = "### Step 7\n\n> First.\n> Second.\n> Third.\n" + blocks = extract_blockquote_sections(sample) + assert len(blocks) == 1 + heading, body = blocks[0] + assert heading == "Step 7" + assert isinstance(heading, str) + assert isinstance(body, str) + assert "First." in body + + +# --- extract_blockquote_placeholders --- + + +def test_extract_blockquote_placeholders_extracts_content_suffix_vars() -> None: + """Given a blockquote with {annotated_diff_content} and {diff_content}, return both names.""" + text = "Review the following:\n{annotated_diff_content}\nand {diff_content} here.\n" + placeholders = extract_blockquote_placeholders(text) + assert placeholders == {"annotated_diff_content", "diff_content"} + + +def test_extract_blockquote_placeholders_ignores_non_content_vars() -> None: + """A blockquote with only {*_path} vars (no {*_content}) returns an empty set.""" + text = "Read {annotated_diff_path}.\nInspect {diff_path}.\n" + placeholders = extract_blockquote_placeholders(text) + assert placeholders == set() From 4efd9441b94b76efa368eafae119dc2f23d3b742 Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 18 Jul 2026 17:50:58 -0700 Subject: [PATCH 2/9] feat(recipe): implement inline-content-in-subagent-prompt semantic rule Add a new @semantic_rule that fires WARNING when a run_skill step's SKILL.md has a blockquoted subagent prompt containing a banned {*_content} placeholder. Detection follows the existing _check_undefined_bash_placeholder resolution pattern: iterate recipe steps, resolve skill_command to skill_name, read SKILL.md, then scan for banned placeholders via the new extract_blockquote_sections and extract_blockquote_placeholders helpers. Banned placeholders (initial set): - {annotated_diff_content} - {diff_content} - {section_diff_content} These are wrong-shape: subagent prompts must reference content by PATH (*_path) and instruct the subagent to Read the file, per the inline-content-in-subagent-prompt architectural rule (PR #3651). The existing xfail-strict test test_inline_content_rule_fires_for_banned_var is un-xfailed; the rule now exists and the test passes. Also update recipe/AGENTS.md description of _skill_placeholder_parser.py to include 'blockquote section extraction' alongside the existing helpers. Known side effect (intentional): the rule will fire WARNING on the existing review-pr, review-research-pr, and audit-claims SKILL.md files on run_semantic_rules(). Part B will migrate those SKILL.md files and remove the rule's findings. Co-Authored-By: Claude --- src/autoskillit/recipe/AGENTS.md | 2 +- .../recipe/rules/rules_skill_content.py | 71 +++++++++++++++++++ .../test_inline_content_semantic_rule.py | 4 -- 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/src/autoskillit/recipe/AGENTS.md b/src/autoskillit/recipe/AGENTS.md index 127fe3f511..c4bedec01b 100644 --- a/src/autoskillit/recipe/AGENTS.md +++ b/src/autoskillit/recipe/AGENTS.md @@ -42,7 +42,7 @@ Sub-package: rules/ (see rules/AGENTS.md). | `_analysis_detectors.py` | Dead outputs + ref invalidations + implicit handoffs | | `_git_helpers.py` | Shared git-remote regex (`_GIT_REMOTE_COMMAND_RE`, `_LITERAL_ORIGIN_RE`) for lint rules | | `_skill_helpers.py` | Shared helpers for skill-related semantic rules | -| `_skill_placeholder_parser.py` | SKILL.md structural analysis: bash placeholder extraction, step section parsing, section splitting, prose GraphQL detection, git command extraction | +| `_skill_placeholder_parser.py` | SKILL.md structural analysis: bash placeholder extraction, step section parsing, section splitting, prose GraphQL detection, git command extraction, blockquote section extraction | | `identity.py` | Recipe identity hashing — content and composite fingerprints | | `schema.py` | `Recipe`, `RecipeStep`, `DataFlowWarning` | | `staleness_cache.py` | Staleness cache for contract and diagram freshness checks | diff --git a/src/autoskillit/recipe/rules/rules_skill_content.py b/src/autoskillit/recipe/rules/rules_skill_content.py index ffa09bb098..a58473c83a 100644 --- a/src/autoskillit/recipe/rules/rules_skill_content.py +++ b/src/autoskillit/recipe/rules/rules_skill_content.py @@ -17,6 +17,8 @@ _VRULE_RE, extract_bash_blocks, extract_bash_placeholders, + extract_blockquote_placeholders, + extract_blockquote_sections, extract_declared_ingredients, extract_graphql_blocks, extract_python_blocks, @@ -960,3 +962,72 @@ def _check_graphql_query_requires_shell_invocation(ctx: ValidationContext) -> li ) return findings + + +_BANNED_BLOCKQUOTE_VARS: frozenset[str] = frozenset( + { + "annotated_diff_content", + "diff_content", + "section_diff_content", + } +) + + +@semantic_rule( + name="inline-content-in-subagent-prompt", + description=( + "A SKILL.md blockquoted subagent prompt references a banned *_content " + "variable. Subagent prompts must use *_path variables and instruct the " + "subagent to Read the file, per the inline-content-in-subagent-prompt " + "architectural rule (PR #3651)." + ), +) +def _check_inline_content_in_subagent_prompt(ctx: ValidationContext) -> list[RuleFinding]: + """Fire WARNING when a run_skill step's SKILL.md uses banned *_content placeholders. + + Detection scans blockquote subagent prompts for ``{*_content}`` variables + that are never defined as ingredients. The naming convention is wrong: + subagent prompts must reference content by PATH (``*_path``) with the + subagent reading the file. Inline content placeholders are dangling and + will be silently dropped by the recipe framework, leaving the subagent + prompt incomplete. + """ + findings: list[RuleFinding] = [] + for step_name, step in ctx.recipe.steps.items(): + if step.tool != "run_skill": + continue + skill_cmd = step.with_args.get("skill_command", "") + if not skill_cmd: + continue + skill_name = resolve_skill_name(skill_cmd) + if skill_name is None: + continue + skill_md = _resolve_skill_md(skill_name, resolver=ctx.skill_resolver) + if skill_md is None: + continue # unknown-skill-command rule handles missing skills + try: + content = skill_md.read_text(encoding="utf-8") + except OSError: + continue # file deleted or unreadable between resolution and read + + for step_context, block_text in extract_blockquote_sections(content): + banned_found = extract_blockquote_placeholders(block_text) & _BANNED_BLOCKQUOTE_VARS + if not banned_found: + continue + for banned_var in sorted(banned_found): + context_label = f"step {step_context!r}" if step_context else "no step heading" + findings.append( + make_finding( + rule_name="inline-content-in-subagent-prompt", + step_name=step_name, + message=( + f"Skill '{skill_name}' blockquote subagent prompt in {context_label} " + f"references banned {{placeholder}} {{{banned_var}}}. " + f"Subagent prompts must use a *_path variable (e.g., " + f"{{{banned_var.removesuffix('_content')}_path}}) and instruct the " + f"subagent to Read the file. Inline content placeholders are " + f"never populated and produce incomplete prompts." + ), + ) + ) + return findings diff --git a/tests/recipe/test_inline_content_semantic_rule.py b/tests/recipe/test_inline_content_semantic_rule.py index 7b39776463..1f74873fec 100644 --- a/tests/recipe/test_inline_content_semantic_rule.py +++ b/tests/recipe/test_inline_content_semantic_rule.py @@ -52,10 +52,6 @@ def _make_skill_md_with_blockquote(var: str) -> str: ) -@pytest.mark.xfail( - reason="inline-content-in-subagent-prompt rule not yet implemented (#3636 prerequisite)", - strict=True, -) @pytest.mark.parametrize( "banned_var", [ From 49a1e829c5e971b254827ae10ab79dc116c03cef Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 18 Jul 2026 17:51:08 -0700 Subject: [PATCH 3/9] fix(execution): filter_findings null-safety for null/missing/zero line numbers filter_findings() previously crashed with TypeError on findings with explicit 'line': None (None in valid_set or start <= None <= end), and silently demoted them to 'filtered' in the validation-absent branch. Two changes: 1. Early-return branch: partition findings with null/zero line to unpostable before returning FilterResult. Null-line findings are NEVER treated as postable, even when no validation data exists. 2. Loop body: change 'line_num = finding.get("line", 0)' to 'line_num = finding.get("line") or 0' and route line_num == 0 directly to unpostable. Avoids the None in valid_set / start <= None comparison TypeError. Tests cover: - null line routes to unpostable (with valid_ranges) - missing 'line' key routes to unpostable - line: 0 routes to unpostable - null line NOT in filtered when validation absent (early-return branch) - null line against hunk-range branch does not raise TypeError Co-Authored-By: Claude --- src/autoskillit/execution/diff_annotator.py | 12 ++++- tests/execution/test_diff_annotator.py | 56 +++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/autoskillit/execution/diff_annotator.py b/src/autoskillit/execution/diff_annotator.py index bab041d961..c418f2fc90 100644 --- a/src/autoskillit/execution/diff_annotator.py +++ b/src/autoskillit/execution/diff_annotator.py @@ -202,13 +202,18 @@ def filter_findings( """Partition findings into filtered (in-range) and unpostable (out-of-range). When valid_lines is provided, uses exact set-membership for validation instead - of hunk-span interval checking. When both are absent, all findings pass through. + of hunk-span interval checking. When both are absent, all findings pass through + except those with null/zero line numbers, which route to unpostable. Sets all_unpostable=True when total findings > 0 and filtered is empty. """ if not findings: return FilterResult() if not valid_ranges and valid_lines is None: + good = [f for f in findings if f.get("line") and f["line"] != 0] + bad = [f for f in findings if not f.get("line") or f["line"] == 0] + if bad: + return FilterResult(filtered=good, unpostable=bad, all_unpostable=not good) return FilterResult(filtered=list(findings)) filtered: list[dict] = [] @@ -216,7 +221,10 @@ def filter_findings( for finding in findings: file_path = finding.get("file", "") - line_num = finding.get("line", 0) + line_num = finding.get("line") or 0 + if line_num == 0: + unpostable.append(finding) + continue if valid_lines is not None: valid_set = set(valid_lines.get(file_path, [])) diff --git a/tests/execution/test_diff_annotator.py b/tests/execution/test_diff_annotator.py index 1240c783ce..29b53c10c7 100644 --- a/tests/execution/test_diff_annotator.py +++ b/tests/execution/test_diff_annotator.py @@ -191,6 +191,62 @@ def test_no_findings_no_failure(self): result = filter_findings([], {"f.py": [(10, 20)]}) assert result.all_unpostable is False + def test_filter_findings_null_line_routes_to_unpostable(self): + """A finding with `"line": None` goes to `unpostable` and does not raise + (with non-empty `valid_ranges`).""" + ranges = {"f.py": [(10, 20)]} + findings = [{"file": "f.py", "line": None, "message": "null line"}] + result = filter_findings(findings, ranges) + assert len(result.filtered) == 0 + assert len(result.unpostable) == 1 + assert result.unpostable[0]["line"] is None + assert result.all_unpostable is True + + def test_filter_findings_missing_line_key_routes_to_unpostable(self): + """A finding without a `line` key goes to `unpostable`.""" + ranges = {"f.py": [(10, 20)]} + findings = [{"file": "f.py", "message": "missing line"}] + result = filter_findings(findings, ranges) + assert len(result.filtered) == 0 + assert len(result.unpostable) == 1 + assert "line" not in result.unpostable[0] + + def test_filter_findings_zero_line_routes_to_unpostable(self): + """A finding with `"line": 0` goes to `unpostable` (valid lines start at 1).""" + ranges = {"f.py": [(10, 20)]} + findings = [{"file": "f.py", "line": 0, "message": "zero line"}] + result = filter_findings(findings, ranges) + assert len(result.filtered) == 0 + assert len(result.unpostable) == 1 + assert result.unpostable[0]["line"] == 0 + + def test_filter_findings_null_line_not_in_filtered_when_validation_absent(self): + """A finding with `"line": None` is NOT placed in `filtered` even when both + `valid_ranges={}` and `valid_lines=None` (the early-return branch).""" + findings = [ + {"file": "f.py", "line": None, "message": "null line"}, + {"file": "f.py", "line": 15, "message": "valid"}, + ] + result = filter_findings(findings, {}) + assert len(result.filtered) == 1 + assert result.filtered[0]["line"] == 15 + assert len(result.unpostable) == 1 + assert result.unpostable[0]["line"] is None + assert result.all_unpostable is False + + def test_filter_findings_null_line_hunk_range_branch_no_typeerror(self): + """A finding with `"line": None` against the hunk-range interval branch + (`valid_ranges={"f.py": [(10, 20)]}`, `valid_lines=None`) routes to + `unpostable` without raising `TypeError`. Exercises the + `start <= line_num <= end` comparison path that previously crashed on + `None`.""" + ranges = {"f.py": [(10, 20)]} + findings = [{"file": "f.py", "line": None, "message": "null line"}] + result = filter_findings(findings, ranges) + assert len(result.filtered) == 0 + assert len(result.unpostable) == 1 + assert result.unpostable[0]["line"] is None + # --- End-to-end --- From 22d5ac88c59c16320ddc11addcbcc5d7ce4755d1 Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 18 Jul 2026 17:56:20 -0700 Subject: [PATCH 4/9] fix(tests): add rules_skill_content line-limit exemption and bump recipe maxima for inline-content rule warnings - tests/arch/test_subpackage_isolation.py: Add REQ-CNST-010-E11 exemption for rules_skill_content.py (1033 lines, just above 1000-line REQ-CNST-010 limit). The kitchen-sink semantic-rules file co-locates all SKILL.md content validators (placeholder, git, write, attribution, inline-content) to keep @semantic_rule registration a single import point. Splitting would fragment the discovery surface and break the test filter cascade. - tests/infra/test_pretty_output_recipe.py: Bump canonical maxima snapshot to reflect new inline-content-in-subagent-prompt rule findings (load_recipe 183585, open_kitchen 183644). New rule emits Severity.WARNING findings for review-pr, review-research-pr, and audit-claims SKILL.md files per plan Part B; the legitimate +826-byte growth is absorbed within RESPONSE_BACKSTOP ceilings (185000/186000). --- tests/arch/test_subpackage_isolation.py | 17 +++++++++++++++++ tests/infra/test_pretty_output_recipe.py | 4 ++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/arch/test_subpackage_isolation.py b/tests/arch/test_subpackage_isolation.py index 69926f7f2f..3bf9629b09 100644 --- a/tests/arch/test_subpackage_isolation.py +++ b/tests/arch/test_subpackage_isolation.py @@ -1091,6 +1091,23 @@ def test_data_directories_are_not_python_packages() -> None: "; _register_agent_tomls session-config registration for generated roles " "(+39 net lines)", ), + "rules_skill_content.py": ( + 1200, + "REQ-CNST-010-E11: SKILL.md content validation rules registry — accumulating " + "semantic rules (undefined-bash-placeholder, hardcoded-origin-remote, " + "blind-git-add, no-interpreter-mediated-writes, no-autoskillit-import, " + "no-posix-char-class, no-grep-bre-alternation, output-section-no-markdown-directive, " + "no-gh-issue-comment, transition-boundary-anti-confirmation, " + "executable-field-content-validity, reviews-post-requires-input-flag, " + "source-attribution-directive, graphql-query-requires-shell-invocation, " + "inline-content-in-subagent-prompt) co-located to keep SKILL.md validation " + "discovery a single import; splitting into sub-modules per rule would fragment " + "the @semantic_rule registration surface and break the test filter cascade." + "; inline-content-in-subagent-prompt rule (#4289 manifestation, #3636 architectural): " + "extract_blockquote_sections + extract_blockquote_placeholders helpers co-located " + "in _skill_placeholder_parser.py and re-used by both rules_skill_content.py " + "and the tests/skills/ contract linters (+~60 net lines)", + ), } diff --git a/tests/infra/test_pretty_output_recipe.py b/tests/infra/test_pretty_output_recipe.py index 1f9e73b82e..5e68fc0106 100644 --- a/tests/infra/test_pretty_output_recipe.py +++ b/tests/infra/test_pretty_output_recipe.py @@ -1052,8 +1052,8 @@ def test_canonical_recipe_responses_fit_independent_registry_ceilings(tmp_path, for ingredients_only in (False, True) } assert maxima == { - "load_recipe": (182_759, "remediation", "all_truthy"), - "open_kitchen": (182_818, "remediation", "all_truthy"), + "load_recipe": (183_585, "remediation", "all_truthy"), + "open_kitchen": (183_644, "remediation", "all_truthy"), } From 754da1a7ce2a69442702f5289f299b1d88f2edcb Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 18 Jul 2026 18:31:57 -0700 Subject: [PATCH 5/9] fix(recipe): migrate review-pr subagent prompts to *_path variables (#4289) Replace {annotated_diff_content} and {diff_content} placeholders in the Step 3 subagent prompts with path-based Read instructions targeting the {annotated_diff_path} and {diff_file_path} arguments. The annotated diff path is forwarded by the recipe from the optional annotate_pr_diff step; when unset or missing, the orchestrating agent falls back to evaluating the diff without [LNNN] anchors (mirroring the existing DISPATCH_AGENTS graceful-degradation fallback in Step 2.9). Also add a NEVER block prohibition against embedding diff content inline in subagent prompts, and update the Step 3 prose to reference the new path-based pattern. Closes the {annotated_diff_content} and {diff_content} dangling-variable findings for the review-pr skill. --- src/autoskillit/skills_extended/review-pr/SKILL.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/autoskillit/skills_extended/review-pr/SKILL.md b/src/autoskillit/skills_extended/review-pr/SKILL.md index d384542730..b044a44c76 100644 --- a/src/autoskillit/skills_extended/review-pr/SKILL.md +++ b/src/autoskillit/skills_extended/review-pr/SKILL.md @@ -50,6 +50,7 @@ by the recipe pipeline after `open_pr_step` opens the PR. - Run subagents in the background (`run_in_background: true` is prohibited) - Issue subagent Task calls sequentially — ALL must be in a single parallel message - Specify `subagent_type` when spawning audit subagents — these are ephemeral agents, not registered agent definitions. Use `Agent(model="sonnet")` exactly as shown, with no `subagent_type` parameter. +- Embed diff content inline in subagent prompts — always pass by path and instruct subagents to Read **ALWAYS:** - Find the PR by feature branch at invocation time (not from a pre-captured URL) @@ -410,8 +411,9 @@ Subagent prompt template (dimensions 1–6): > `+` or context line's marker in the same hunk. > > If no issues found, return an empty array []. -> Annotated diff content (each line prefixed with [LNNN] markers): -> {annotated_diff_content} +> Read the annotated diff file at path: {annotated_diff_path} +> Each line in the file is prefixed with [LNNN] markers indicating the GitHub diff line number. +> Use the [LNNN] number as the `line` value in your findings JSON. > > Prior resolved findings (DO NOT RE-RAISE — these have been addressed by resolve-review): > {json_list_of_prior_resolved_findings or "[]"} @@ -442,7 +444,11 @@ Subagent prompt template (dimensions 1–6): > are repetitive. Pass `prior_resolved_findings` and `prior_unresolved_findings` (both as JSON arrays) into each -subagent prompt via template substitution, same as `annotated_diff_content`. +subagent prompt via template substitution. The annotated diff is available at `annotated_diff_path` +— instruct subagents to Read it. If `annotated_diff_path` is unset or the file does not exist, +state "No annotated diff is available for this run — evaluate the diff without [LNNN] anchors and +approximate `line` from context" instead of emitting a reference to a nonexistent path. This +mirrors the existing graceful-degradation fallback for `DISPATCH_AGENTS` documented in Step 2.9. Subagent prompt template (dimension 7 — deletion_regression, only when `deletion_context` is non-null): @@ -455,7 +461,7 @@ Subagent prompt template (dimension 7 — deletion_regression, only when `deleti > Deleted symbols: {deletion_context.deleted_symbols} > > PR diff: -> {diff_content} +> Read the raw diff file at path: {diff_file_path} > > Instructions: > - For each deleted file in the deletion context: check if the diff adds or recreates it From ce8b2bfcc5379cb60eea671c5f3df4abb783cfe2 Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 18 Jul 2026 18:32:02 -0700 Subject: [PATCH 6/9] fix(recipe): migrate review-research-pr subagent prompts to annotated_diff_path (#4289) Replace the dangling {annotated_diff_content} placeholder in the Step 3 subagent prompt with a path-based Read instruction targeting the {annotated_diff_path} argument. When unset or missing, the orchestrating agent falls back to evaluating the diff without [LNNN] anchors. Add annotated_diff_path to the Arguments section (the recipe already forwards the variable from the optional annotate_pr_diff step in research-review.yaml, but the skill docs omitted it). Add a NEVER block prohibition against embedding diff content inline in subagent prompts. Closes the {annotated_diff_content} dangling-variable finding for the review-research-pr skill. --- .../skills_extended/review-research-pr/SKILL.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/autoskillit/skills_extended/review-research-pr/SKILL.md b/src/autoskillit/skills_extended/review-research-pr/SKILL.md index a30f19fe22..713b6fadbf 100644 --- a/src/autoskillit/skills_extended/review-research-pr/SKILL.md +++ b/src/autoskillit/skills_extended/review-research-pr/SKILL.md @@ -20,12 +20,13 @@ a summary verdict. Called by the recipe pipeline after `open_research_pr` opens ## Arguments -`/autoskillit:review-research-pr [hunk_ranges_path=] [valid_lines_path=]` +`/autoskillit:review-research-pr [annotated_diff_path=] [hunk_ranges_path=] [valid_lines_path=]` - **worktree-path-or-feature-branch** — Either an absolute path to the research worktree (preferred; skill derives the feature branch from `git rev-parse --abbrev-ref HEAD`) or the feature branch name directly - **base-branch** — The base branch the PR targets (e.g., "main") +- **annotated_diff_path** (optional) — absolute path to a pre-computed annotated diff file (produced by `annotate_pr_diff` run_python step). When provided and present, read from file instead of running python3. - **hunk_ranges_path** (optional) — absolute path to a pre-computed hunk ranges JSON file (produced by `annotate_pr_diff` run_python step). When provided, loaded in Step 2.7 instead of parsing from the diff inline. - **valid_lines_path** (optional) — absolute path to a pre-computed valid lines JSON file (produced by `annotate_pr_diff` run_python step). Contains exact `{filepath: [line_numbers]}` set. When provided, enables exact set-membership validation in Step 4. @@ -48,6 +49,7 @@ a summary verdict. Called by the recipe pipeline after `open_research_pr` opens results are valid outcomes for research PRs (do not flag them) - Run subagents in the background (`run_in_background: true` is prohibited) - Issue subagent Task calls sequentially — ALL must be in a single parallel message +- Embed diff content inline in subagent prompts — always pass by path and instruct subagents to Read **ALWAYS:** - Find the PR by feature branch at invocation time (not from a pre-captured URL) @@ -226,8 +228,12 @@ Subagent prompt template (all 8 dimensions): > `+` or context line's marker in the same hunk. > > If no issues found, return an empty array []. -> Annotated diff content (each line prefixed with [LNNN] markers): -> {annotated_diff_content} +> Read the annotated diff file at path: {annotated_diff_path} +> Each line is prefixed with [LNNN] markers. Use the [LNNN] number as the `line` value. + +If `annotated_diff_path` is unset or the file does not exist, state "No annotated diff is +available for this run — evaluate the diff without [LNNN] anchors and approximate `line` from +context" instead of emitting a reference to a nonexistent path. ### Step 4: Aggregate and Deduplicate Findings From 34e719fd81c8044ef02ac3ee08d21c4c8401e634 Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 18 Jul 2026 18:32:04 -0700 Subject: [PATCH 7/9] fix(recipe): migrate audit-claims subagent prompts to *_path variables (#4289) Phase 1 (claim extraction): insert a new instruction before the "Launch one subagent" line teaching the orchestrating agent to write each section's diff chunk to its own file at {{AUTOSKILLIT_TEMP}}/audit-claims/section_diff_{section_slug}_{pr_number}.txt and bind {section_diff_path} to that file path when building each subagent's prompt. Replace the dangling {section_diff_content} placeholder with a Read instruction. Phase 2 (evidence matching): replace {claims_json} with a Read from {claims_json_path} and replace {diff_content} with a Read from {diff_file_path} (both resolve to concrete paths already documented in this skill). Replace the dangling {evidence_rules_for_type} with inline referral to the static per-claim-type rules already enumerated in the skill above the template. Update the Temp File Layout section to enumerate the new section_diff intermediate files. Add a NEVER block prohibition against embedding diff content inline in subagent prompts. Closes the {section_diff_content}, {claims_json}, {diff_content}, and {evidence_rules_for_type} dangling-variable findings for the audit-claims skill. --- .../skills_extended/audit-claims/SKILL.md | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/autoskillit/skills_extended/audit-claims/SKILL.md b/src/autoskillit/skills_extended/audit-claims/SKILL.md index 0670d518c2..6745c650b9 100644 --- a/src/autoskillit/skills_extended/audit-claims/SKILL.md +++ b/src/autoskillit/skills_extended/audit-claims/SKILL.md @@ -51,6 +51,7 @@ comments and emits a verdict for recipe routing. - Generate findings for `experimental` claims — they are self-evidencing by definition - Run subagents in the background (`run_in_background: true` is prohibited) - Issue subagent Task calls sequentially — ALL must be in a single parallel message +- Embed diff content inline in subagent prompts — always pass by path and instruct subagents to Read **ALWAYS:** - Use the explicit `pr_url` argument instead of re-discovering via `gh pr list` @@ -119,6 +120,14 @@ Do not output any prose between subagent dispatches. Immediately proceed to the Divide the diff by top-level markdown section: `## Executive Summary`, `## Results`, `## Methodology`, `## Discussion`, `## Limitations`, and any other top-level `##` section. +After dividing the diff by section, write each section's diff chunk to its own file at +`{{AUTOSKILLIT_TEMP}}/audit-claims/section_diff_{section_slug}_{pr_number}.txt` where +`{section_slug}` is the section name lowercased with spaces replaced by underscores (e.g. +`executive_summary`, `results`, `methodology`). Use `jq -n` or the Write tool — do not use +inline Python one-liners or heredoc scripts with `open()` (these are blocked by the sandbox, +per the existing convention at Step 1 of this file). Then bind `{section_diff_path}` to that +section's file path when building each subagent's prompt. + Launch one subagent via `Agent(model="sonnet")` per section containing `+` diff lines. Each subagent returns a JSON array of extracted claims: @@ -154,8 +163,7 @@ Subagent prompt template: > - comparative: compares to prior work, published results, or baselines > > If no claims found in this section, return an empty array []. -> Diff content for section [{section_name}]: -> {section_diff_content} +> Read the section diff from: {section_diff_path} Aggregate all extracted claims from all subagents. Save to `{{AUTOSKILLIT_TEMP}}/audit-claims/claims_{pr_number}.json`. Use `jq -n` or the Write tool. If the file already exists from a prior retry, either read it first (to satisfy the Write tool guard) or use a Bash redirect (`jq -n ... > path`). @@ -189,6 +197,12 @@ returns findings: - `comparative` — requires attribution; "comparable to state-of-the-art" without citation is `critical` +When building each subagent's prompt, bind: +- `{claims_json_path}` to `{{AUTOSKILLIT_TEMP}}/audit-claims/claims_{pr_number}.json` + (the file written in Phase 1 above) +- `{diff_file_path}` to `{{AUTOSKILLIT_TEMP}}/audit-claims/diff_{pr_number}.txt` + (the file written in Step 2 above) + Subagent prompt template: > You are checking citation evidence for [{claim_type}] claims in a GitHub PR diff. @@ -203,13 +217,17 @@ Subagent prompt template: > remove claim). > > Evidence rules for [{claim_type}]: -> {evidence_rules_for_type} +> Apply the rule for your claim type as enumerated under "Evidence rules per claim type" +> earlier in this skill (skipped entirely for `experimental` claims; warning for missing +> `external` citations or for specific numeric comparisons in `external` claims being +> critical; warning for missing `methodological` rationale; critical for unattributed +> `comparative` claims such as "comparable to state-of-the-art" without citation). > > If all claims have adequate evidence, return an empty array []. > Claims to check: -> {claims_json} +> Read the claims file at: {claims_json_path} > Full PR diff: -> {diff_content} +> Read the diff file at: {diff_file_path} Save findings to `{{AUTOSKILLIT_TEMP}}/audit-claims/findings_{pr_number}.json`. Use `jq -n` or the Write tool. If the file already exists from a prior retry, either read it first (to satisfy the Write tool guard) or use a Bash redirect (`jq -n ... > path`). @@ -410,6 +428,7 @@ Exit 1 only for unrecoverable tool-level errors. ``` {{AUTOSKILLIT_TEMP}}/audit-claims/ ├── diff_{pr_number}.txt +├── section_diff_{section_slug}_{pr_number}.txt (Phase 1 intermediate, one per section) ├── claims_{pr_number}.json (Phase 1 output) ├── findings_{pr_number}.json (Phase 2 output) └── summary_{pr_number}_{ts}.md From 2fec702e1f7681552cd78aa2f8c1a09c1021396b Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 18 Jul 2026 18:32:09 -0700 Subject: [PATCH 8/9] fix(tests): un-xfail review-pr diff annotation contracts after Part B migration (#4289) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the six xfail(strict=True) markers and the three skipif(extract_blockquote_sections is None) guards that were bridging the Part A prerequisite gap. Convert the try/except ImportError fallback for extract_blockquote_sections to a direct import — the function now exists at the expected path. These contracts assert that the review-pr, review-research-pr, and audit-claims SKILL.md blockquote subagent prompts use *_path variables (instructing subagents to Read the content from disk) rather than inline *_content variables, and that each skill's NEVER block contains an explicit prohibition against inline diff embedding. The three SKILL.md migrations in this PR satisfy both predicates, so the xfail/bridge markers are no longer needed. Closes the contract-test gating for the Part B SKILL.md migrations. --- .../test_review_pr_diff_annotation.py | 30 +------------------ 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/tests/contracts/test_review_pr_diff_annotation.py b/tests/contracts/test_review_pr_diff_annotation.py index 23a21e0c9c..341055439e 100644 --- a/tests/contracts/test_review_pr_diff_annotation.py +++ b/tests/contracts/test_review_pr_diff_annotation.py @@ -7,13 +7,9 @@ import pytest from autoskillit.core.io import load_yaml +from autoskillit.recipe._skill_placeholder_parser import extract_blockquote_sections from autoskillit.recipe.io import builtin_recipes_dir, load_recipe -try: - from autoskillit.recipe._skill_placeholder_parser import extract_blockquote_sections -except ImportError: - extract_blockquote_sections = None # type: ignore[assignment,misc] - pytestmark = [pytest.mark.layer("contracts"), pytest.mark.medium] _CONTRACTS_YAML = Path(__file__).parents[2] / "src/autoskillit/recipe/skill_contracts.yaml" @@ -177,34 +173,24 @@ def test_review_pr_never_specify_subagent_type() -> None: ) -@pytest.mark.xfail(reason="SKILL.md NEVER block not yet updated (#3636 prerequisite)", strict=True) def test_review_pr_never_embed_inline_in_never_block() -> None: """review-pr NEVER block must prohibit inline diff embedding.""" content = _SKILLS_EXTENDED.joinpath("review-pr", "SKILL.md").read_text() assert "Embed diff content inline" in content -@pytest.mark.xfail(reason="SKILL.md NEVER block not yet updated (#3636 prerequisite)", strict=True) def test_review_research_pr_never_embed_inline_in_never_block() -> None: """review-research-pr NEVER block must prohibit inline diff embedding.""" content = _SKILLS_EXTENDED.joinpath("review-research-pr", "SKILL.md").read_text() assert "Embed diff content inline" in content -@pytest.mark.xfail(reason="SKILL.md NEVER block not yet updated (#3636 prerequisite)", strict=True) def test_audit_claims_never_embed_inline_in_never_block() -> None: """audit-claims NEVER block must prohibit inline diff embedding.""" content = _SKILLS_EXTENDED.joinpath("audit-claims", "SKILL.md").read_text() assert "Embed diff content inline" in content -@pytest.mark.skipif( - extract_blockquote_sections is None, - reason="extract_blockquote_sections not yet implemented (#3636 prerequisite)", -) -@pytest.mark.xfail( - reason="SKILL.md blockquote variables not yet migrated (#3636 prerequisite)", strict=True -) def test_review_pr_blockquote_uses_path_not_content_var() -> None: """review-pr blockquotes must use path var, not content var.""" content = _SKILLS_EXTENDED.joinpath("review-pr", "SKILL.md").read_text() @@ -214,13 +200,6 @@ def test_review_pr_blockquote_uses_path_not_content_var() -> None: assert "{annotated_diff_content}" not in all_block_text -@pytest.mark.skipif( - extract_blockquote_sections is None, - reason="extract_blockquote_sections not yet implemented (#3636 prerequisite)", -) -@pytest.mark.xfail( - reason="SKILL.md blockquote variables not yet migrated (#3636 prerequisite)", strict=True -) def test_review_research_pr_blockquote_uses_path_not_content_var() -> None: """review-research-pr blockquotes must use path var, not content var.""" content = _SKILLS_EXTENDED.joinpath("review-research-pr", "SKILL.md").read_text() @@ -230,13 +209,6 @@ def test_review_research_pr_blockquote_uses_path_not_content_var() -> None: assert "{annotated_diff_content}" not in all_block_text -@pytest.mark.skipif( - extract_blockquote_sections is None, - reason="extract_blockquote_sections not yet implemented (#3636 prerequisite)", -) -@pytest.mark.xfail( - reason="SKILL.md blockquote variables not yet migrated (#3636 prerequisite)", strict=True -) def test_audit_claims_blockquote_uses_path_not_content_var() -> None: """audit-claims blockquotes must use path var, not content var.""" content = _SKILLS_EXTENDED.joinpath("audit-claims", "SKILL.md").read_text() From 80c69709bce6edde0a65ef839ce41d95b2644355 Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 18 Jul 2026 18:38:53 -0700 Subject: [PATCH 9/9] fix: refresh recipe response ceiling snapshot --- tests/infra/test_pretty_output_recipe.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/infra/test_pretty_output_recipe.py b/tests/infra/test_pretty_output_recipe.py index 5e68fc0106..1f9e73b82e 100644 --- a/tests/infra/test_pretty_output_recipe.py +++ b/tests/infra/test_pretty_output_recipe.py @@ -1052,8 +1052,8 @@ def test_canonical_recipe_responses_fit_independent_registry_ceilings(tmp_path, for ingredients_only in (False, True) } assert maxima == { - "load_recipe": (183_585, "remediation", "all_truthy"), - "open_kitchen": (183_644, "remediation", "all_truthy"), + "load_recipe": (182_759, "remediation", "all_truthy"), + "open_kitchen": (182_818, "remediation", "all_truthy"), }