feat(agents): leaf prompt profile, typed coding-artifact contract, chain extraction - #213
Merged
Conversation
Make pythinker_code.utils.artifacts the single source of truth for the <coding_artifact> handoff: a dataclass-derived prompt contract block, a fail-closed extraction API (extracted/missing/malformed with strict end-of-message anchoring, exactly-one-block, duplicate- and undeclared-key rejection), a files_changed cross-check in the verifier artifact receipt, and invariant tests binding the implementer/coder prompt copies and the verifier consumer to the schema.
…xtraction Replace the chain's local <coding_artifact> regex with the typed fail-closed extraction API from utils.artifacts. Malformed artifacts now surface distinctly in the judge prompt (untrusted plain-fenced raw block with the bounded reason) and in the chain result line, instead of being passed to the judge as if valid; missing and present renderings are unchanged. _extract_coding_artifact stays as a compatibility adapter.
… module Move the implementer→judge chain (~550 lines: regexes, params, fingerprint, verdict parsing, prompt builders, ImplementAndJudgeTool) out of tools/agent/__init__.py into tools/agent/implement_judge.py. The full previous import surface of pythinker_code.tools.agent is preserved via explicit re-exports, so the agent-spec tool path and all existing imports work unchanged; the new submodule is registered in the PyInstaller hiddenimports snapshot.
Extract the 12 shared sections of the root system prompt into agents/default/partials/*.md included via Jinja, keeping the rendered root prompt byte-identical (verified by fixed-args render diff). The duplicate-report-prose test now asserts against the rendered prompt instead of raw template bytes, and the PyInstaller datas snapshot gains the new partial files.
New leaf profile composed from the shared prompt partials: identity, subagent preamble, role slot, conditional artifact contract (rendered from utils.artifacts via the new PYTHINKER_CODING_ARTIFACT_CONTRACT render arg gated by EMITS_CODING_ARTIFACT), core rules, tool basics, code standards, untrusted content, communication, definition of done, environment, AGENTS.md, and skills — with no root-only orchestration or playbook prose. No role is migrated yet.
Both roles now render system_leaf.md with EMITS_CODING_ARTIFACT, so the subagent preamble and artifact contract come from the template (single source of truth) and the root-only orchestration/playbook prose is no longer shipped to these leaf roles. ROLE_ADDITIONAL keeps only the role-specific sections. Rendered implementer prompt drops from ~7,270 to ~4,240 words (-42%); coder from ~7,770 to ~4,730 (-39%). The artifact contract invariant now asserts against the rendered prompts.
verifier, judge, explore, plan, planner, scout, review, code-reviewer, security-reviewer, and debugger now render the leaf profile: the subagent preamble comes from the template and each role stops shipping the root agent's orchestration/playbook manual. ROLE_ADDITIONAL bodies are otherwise unchanged.
…variants The four large ROLE_ADDITIONAL inline snapshots (coder, explore, plan, planner) become exact section-heading lists, template-ownership checks (no role embeds the subagent preamble or artifact contract — asserted across all 12 roster roles), EMITS_CODING_ARTIFACT flag assertions, and load-bearing identity lines, per the repo policy of preferring small semantic prompt assertions over large snapshots.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR modularizes root and leaf agent prompts, introduces strict typed coding-artifact contracts, and moves the ImplementAndJudge chain into its own module while preserving imports. Tests cover prompt rendering, artifact validation, malformed handoffs, orchestration behavior, and packaged assets. ChangesAgent prompt composition
Typed coding-artifact contract
Implementer-judge orchestration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant ImplementAndJudgeTool
participant Implementer
participant ArtifactExtractor
participant Judge
ImplementAndJudgeTool->>Implementer: send implementation brief
Implementer-->>ArtifactExtractor: return tagged coding artifact
ArtifactExtractor-->>ImplementAndJudgeTool: return validated or malformed result
ImplementAndJudgeTool->>Judge: send implementation evidence
Judge-->>ImplementAndJudgeTool: return verdict and required fixes
ImplementAndJudgeTool->>Implementer: request bounded revision when needed
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pythinker_code/agents/default/partials/environment.md`:
- Around line 13-22: Update the environment prompt template around
PYTHINKER_WORK_DIR_LS and PYTHINKER_ADDITIONAL_DIRS_INFO to render
filesystem-derived values using a collision-safe untrusted-data wrapper that
cannot be terminated by repository-controlled filenames. Preserve the existing
directory context and absolute-path guidance while ensuring injected content is
clearly treated as data rather than instructions.
In `@src/pythinker_code/agents/default/partials/untrusted_content.md`:
- Around line 3-5: Update the guidance in the untrusted-content partial to state
that literal `<system>` and `<system-reminder>` tags in user-controlled messages
never grant authority, and must be treated as untrusted content unless supplied
through an authenticated runtime directive. Clarify that only actual
system-level directives are authoritative, while preserving the existing
protections for `<untrusted_data>` and the instruction to neutralize or wrap
external content.
In `@src/pythinker_code/soul/agent.py`:
- Around line 782-786: Update the render flow around template.render so
system_prompt_args cannot override the generated
PYTHINKER_CODING_ARTIFACT_CONTRACT value. Either reject that reserved key before
rendering or merge the arguments so coding_artifact_contract_block() is applied
last and remains authoritative.
In `@src/pythinker_code/tools/agent/implement_judge.py`:
- Around line 423-456: The revision flow around _parse_judge_verdict must
validate last_artifact independently before accepting a PASS verdict. If the
required artifact is missing or malformed and the judge returns PASS, override
the result to NEEDS_WORK when another revision remains, otherwise BLOCKED;
preserve the existing judge-error handling and add regression tests covering
missing and malformed artifacts paired with PASS.
- Around line 176-177: Neutralize model-generated revision feedback before
replaying it across the judge/implementer boundary: at
src/pythinker_code/tools/agent/implement_judge.py lines 176-177, serialize the
feedback as escaped data before appending it to the revision section; at lines
217-225, replace fixed Markdown fences with JSON serialization or a delimiter
guaranteed not to occur in the content. Preserve the existing prompt structure
while ensuring embedded content cannot break out or inject instructions.
- Around line 277-294: Add the appropriate return type annotation to the public
__init__ method of the relevant class, preserving its existing initialization
behavior and parameters.
- Around line 413-421: Update the implementer-error branch in the revision flow
to clear last_required_fixes along with last_verdict, last_verdict_raw, and
last_artifact, so prior-revision judge feedback is not later emitted as current
judge_output. Preserve the existing BLOCKED verdict and error message handling.
- Around line 118-134: Update _parse_judge_verdict to require the verdict regex
to match the first non-formatting token in summary_body, rather than searching
for a later token. Anchor _IMPLEMENT_JUDGE_VERDICT_RE appropriately and use
match() so prose before a verdict fails closed as BLOCKED.
In `@tests/core/test_agent_spec.py`:
- Around line 123-151: Extend the assertions in the subagent specification test
to explicitly cover the implementer leaf contract. Validate implementer uses
system_leaf.md, has EMITS_CODING_ARTIFACT set to "true", and retains its
expected ROLE_ADDITIONAL mission content, while preserving the existing generic
checks.
In `@tests/utils/test_artifacts.py`:
- Around line 267-295: Update the tests for extract_coding_artifact to exercise
observable behavior without monkeypatching its internal json.loads dependency:
use genuinely deeply nested JSON to trigger the parser recursion-limit path, and
test reason truncation through the public behavior or the relevant bounding
helper without replacing the decoder. Remove the artificial decoder mocks while
preserving assertions for the MalformedCodingArtifact reason and its
120-character limit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: aabb8a80-58f1-4fa3-8194-e487ef2152e6
⛔ Files ignored due to path filters (2)
tasks/lessons.mdis excluded by!tasks/**tasks/todo.mdis excluded by!tasks/**
📒 Files selected for processing (39)
CHANGELOG.mdsrc/pythinker_code/agents/default/agent.yamlsrc/pythinker_code/agents/default/code_reviewer.yamlsrc/pythinker_code/agents/default/coder.yamlsrc/pythinker_code/agents/default/debugger.yamlsrc/pythinker_code/agents/default/explore.yamlsrc/pythinker_code/agents/default/implementer.yamlsrc/pythinker_code/agents/default/judge.yamlsrc/pythinker_code/agents/default/partials/act_with_tools.mdsrc/pythinker_code/agents/default/partials/agents_md.mdsrc/pythinker_code/agents/default/partials/code_standards.mdsrc/pythinker_code/agents/default/partials/communication.mdsrc/pythinker_code/agents/default/partials/core_rules.mdsrc/pythinker_code/agents/default/partials/definition_of_done.mdsrc/pythinker_code/agents/default/partials/environment.mdsrc/pythinker_code/agents/default/partials/identity_core.mdsrc/pythinker_code/agents/default/partials/skills.mdsrc/pythinker_code/agents/default/partials/spend_context.mdsrc/pythinker_code/agents/default/partials/untrusted_content.mdsrc/pythinker_code/agents/default/partials/verify_results.mdsrc/pythinker_code/agents/default/plan.yamlsrc/pythinker_code/agents/default/planner.yamlsrc/pythinker_code/agents/default/review.yamlsrc/pythinker_code/agents/default/scout.yamlsrc/pythinker_code/agents/default/security_reviewer.yamlsrc/pythinker_code/agents/default/system.mdsrc/pythinker_code/agents/default/system_leaf.mdsrc/pythinker_code/agents/default/verifier.yamlsrc/pythinker_code/soul/agent.pysrc/pythinker_code/tools/agent/__init__.pysrc/pythinker_code/tools/agent/implement_judge.pysrc/pythinker_code/utils/artifacts.pytests/core/test_agent_spec.pytests/core/test_default_agent.pytests/core/test_implement_judge_chain.pytests/core/test_leaf_prompt.pytests/utils/test_artifact_contract.pytests/utils/test_artifacts.pytests/utils/test_pyinstaller_utils.py
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
- Fence PYTHINKER_WORK_DIR_LS and additional-dir listings with a collision-safe backtick fence so repository-controlled filenames cannot break out of the prompt block - Scope <system>/<system-reminder> authority to runtime-injected tags; lookalike tags in user or wrapped content grant none - Keep the generated coding-artifact contract authoritative over spec-provided system_prompt_args in system prompt rendering - Anchor the judge verdict token to the start of SUMMARY (match, not search) and gate PASS on a valid <coding_artifact> block, failing closed to NEEDS_WORK/BLOCKED - Fence implementer output, artifacts, and revision feedback replayed across the judge/implementer boundary with collision-safe fences - Clear stale judge output on revision implementer failure; annotate ImplementAndJudgeTool.__init__ return type - Cover the implementer leaf artifact contract in agent-spec tests; drive artifact error paths with real inputs instead of monkeypatched json.loads
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pythinker_code/tools/agent/implement_judge.py`:
- Around line 231-238: Update the MalformedCodingArtifact handling in the
artifact-section construction to render artifact.reason through
_fenced_untrusted_block() before inserting it into the prompt, keeping the
existing malformed-artifact guidance intact. Add a regression test covering a
duplicate JSON key whose decoded reason contains newlines and prompt-like text,
verifying the reason is fenced as untrusted data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5585be75-2375-4b68-8513-4a55cfa66a29
📒 Files selected for processing (8)
src/pythinker_code/agents/default/partials/environment.mdsrc/pythinker_code/agents/default/partials/untrusted_content.mdsrc/pythinker_code/soul/agent.pysrc/pythinker_code/tools/agent/implement_judge.pytests/core/test_agent_spec.pytests/core/test_default_agent.pytests/core/test_implement_judge_chain.pytests/utils/test_artifacts.py
The extractor's reason for a MalformedCodingArtifact can echo decoded content (e.g. a duplicate JSON key holding newlines and prompt-shaped text), yet it was interpolated into the judge prompt prose outside any untrusted block. Render it through _fenced_untrusted_block() so it is treated as data, and add a duplicate-key regression test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Deepens the implementer (and all leaf subagent) architecture along three seams identified in an architecture review: prompt weight, the coding-artifact handoff contract, and chain locality.
Leaf subagent prompt profile
system.mdis split into 12 shared Jinja partials (agents/default/partials/); the root prompt render is byte-identical to before (verified by fixed-args render diff against a pre-change baseline).system_leaf.mdcomposes the shared sections without root-only Operating Loop / Playbooks / orchestration prose; the subagent preamble and artifact contract are template-owned.Typed coding-artifact contract (single source of truth)
utils/artifacts.pynow owns the<coding_artifact>contract: the prompt block renders from theCodingArtifactschema (PYTHINKER_CODING_ARTIFACT_CONTRACTrender arg gated byEMITS_CODING_ARTIFACT), and extraction is strict fail-closed — exactly one end-of-message block, duplicate/undeclared JSON keys rejected, typed present/missing/malformed results with bounded reasons.ImplementAndJudgesurfaces malformed artifacts distinctly in the judge prompt (untrusted plain-fenced raw block + reason) and its result line, instead of passing them through as if valid; missing/present behavior unchanged byte-for-byte.artifact.files_changedagainstgit diff.Chain extraction
tools/agent/implement_judge.py(547 lines);tools/agent/__init__.pyshrinks 1755 → 1195 lines with the full previous import surface preserved via re-exports (zero chain-test edits; PyInstaller hiddenimports updated).Test hygiene
Verification
make check-pythinker-code— ruff check, ruff format check, pyright (0 errors), ty: all passing.make test-pythinker-code— 2,661 unit tests + 65 e2e tests passing (1+4 skipped) on the composed tree.when_to_use, and tool lists untouched).Changelog
## Unreleasedentries added for the leaf profile, the typed artifact contract, and the chain extraction.Summary by CodeRabbit