feat: ImplementAndJudge chain, per-tool error reporting, and minimum-diff judge rubric

CHANGELOG.md

@@ -15,6 +15,64 @@ GitHub Releases page; `0.8.0` is the new starting line.
 
 ## Unreleased
 
+- **`InvalidToolError` now names the failing tool and the reason.** A bad
+  tool path in `agent.yaml` (typo, missing class, or — most commonly — a
+  `pythinker` binary built before the tool was added) used to surface as a
+  bare `Invalid tools: ['pythinker_code.tools.agent:ImplementAndJudge']`
+  with the actual reason buried in the log file. The aggregated error now
+  lists each bad tool with the per-tool reason, and a class-name miss logs
+  a `Did you mean '<Closest>'?` hint. A constructor exception on one tool
+  is caught per-tool so the user gets one clear error instead of a
+  traceback. Rebuild the binary (`make build-bin`) if the new error names
+  a tool that exists in the working tree.
+- **Auto-chain `implementer` → `judge` for non-trivial scoped edits.** The new
+  `ImplementAndJudge` tool runs `implementer` once, hands the artifact to
+  `judge`, and on `NEEDS_WORK` re-invokes `implementer` once with the
+  judge's `REQUIRED FIXES` under a `## Revision brief` section before
+  re-judging. Two implementer invocations is the hard cap — a
+  still-`NEEDS_WORK` after revision surfaces the contradiction and stops.
+  Parent agents should call `ImplementAndJudge` instead of `Agent:
+  implementer` + `Agent: judge`; bare `judge` calls remain the right shape
+  for non-implementation reviews (reports, audits, severity-scored
+  findings).
+- **Judge adopts the minimum-diff rubric dimension.** Every non-trivial diff
+  the judge reviews is now checked against the reduction ladder
+  (skip-need → reuse-stdlib → use-native → use-installed-dep → one-line →
+  minimum) and the minimum-diff rubric (no abstractions, no new deps
+  without justification, no new config keys without a consumer, no
+  reformatting churn outside the changed lines). The judge applies the full
+  ladder uniformly — there is no mode switch on the judge.
+- **Bundled judge-branded skills.** `judge-minimum-diff` (the rubric
+  applied as a judge dimension) and `judge-overengineering-review` (the
+  parent-facing review checklist) ship as static default skills, replacing
+  ad-hoc prose in the system prompt with explicit, versionable content.
+
+- **Fix OpenAI Responses requests that could still send `role="system"` after
+  switching to a newer Pythinker catalog model (gpt-5.5, gpt-5.3-codex,
+  gpt-5.3-codex-spark, or any user-defined fine-tune).** Pythinker observed
+  OpenAI returning `System messages are not allowed` on this path, but the
+  `system→developer` conversion was previously gated on the openai SDK's
+  `ResponsesModel` literal, which lags Pythinker's own model catalog. The
+  conversion now runs unconditionally in `OpenAIResponses`, so all local
+  system messages are normalized before sending. Also fixes the model-switch
+  carry-over path (`_carry_context_to_session`) whose seeded `role="system"`
+  summary message was sent verbatim on the first request after a switch.
+- **Background bash tasks (npm dev, docker run) no longer show the agent
+  verb spinner.** Pure-bash background work now shows a fixed "Running in
+  background…" label instead of "Composing…/Brewing…" verbs, which read as
+  agent activity. Mixed bash+agent background work keeps the verb spinner
+  while the agent is actively producing tokens.
+- **Quiet background tasks no longer force a 0.1s prompt repaint.** When a
+  background task has produced no output for 2 seconds, the refresh loop
+  drops to the idle 1.0s interval instead of spinning the braille marker at
+  12.5 fps — fixing the "stuck spinner" look for long-running dev servers
+  on Windows VS Code.
+- **Welcome banner no longer shows a stale "Update available" chip after a
+  successful /update.** The banner chip now mirrors the under-input notice:
+  when the update has landed this session (state=UPDATED, smoke check passed),
+  it shows "Updated X → vY. Restart to apply." instead of telling the user
+  to re-run an update that already completed.
+
 ## 0.51.0 (2026-06-22)
 
 - **Reasoning levels now match each GPT model.** The thinking selector and

packages/pythinker-core/src/pythinker_core/contrib/chat_provider/openai_responses.py

@@ -159,10 +159,11 @@ async def generate(
         inputs: ResponseInputParam = []
         instructions = system_prompt if self._system_prompt_as_instructions else None
         if system_prompt and not instructions:
-            system_message: ResponseInputItemParam = {"role": "system", "content": system_prompt}
-            if is_openai_model(self.model_name):
-                system_message["role"] = "developer"
-            inputs.append(system_message)
+            # This class is exclusively the Responses API transport (see provider.type
+            # "openai_responses" / "openai_codex" in pythinker-code). Normalize local
+            # system prompts to developer messages so model-name drift cannot leak an
+            # unsupported system role onto the wire.
+            inputs.append({"role": "developer", "content": system_prompt})
         # The `Message` type is OpenAI-compatible for Responses API `input` messages.
 
         for message in history:
@@ -235,13 +236,13 @@ def _convert_message(self, message: Message) -> list[ResponseInputItemParam]:
 
         Rules:
         - role in {user, assistant}: map to EasyInputMessageParam with role kept
-        role == system: map to role=developer for OpenAI models, otherwise kept
-        content: str kept; list[ContentPart] mapped to ResponseInputMessageContentListParam
+        - role == system: always mapped to role=developer so model-name drift cannot
+          leak a local system role onto the Responses API wire
         - role == tool: map to FunctionCallOutput with call_id and output
         """
 
         role = message.role
-        if is_openai_model(self.model_name) and role == "system":
+        if role == "system":
             role = "developer"
 
         # tool role → function_call_output (return value from a prior tool call)

packages/pythinker-core/tests/api_snapshot_tests/test_openai_responses.py

@@ -3,6 +3,7 @@
 import json
 from typing import Any
 
+import pytest
 import respx
 from common import COMMON_CASES, Case, capture_request, run_test_cases
 from httpx import Response
@@ -531,3 +532,62 @@ async def test_openai_responses_with_thinking_max_clamps_to_xhigh():
             pass
         body = json.loads(mock.calls.last.request.content.decode())
         assert body["reasoning"] == snapshot({"effort": "xhigh", "summary": "auto"})
+
+
+# Regression: local system-prompt roles must become developer messages regardless
+# of model name. A model switch to gpt-5.5 produced
+# `Error code: 400 - {'detail': 'System messages are not allowed'}` while the old
+# conversion gate was tied to the openai SDK's lagging ResponsesModel literal.
+@pytest.mark.parametrize(
+    "model_name",
+    [
+        "gpt-5.5",  # in Pythinker catalog but missing from openai SDK ResponsesModel
+        "gpt-5.3-codex",  # same — in catalog, missing from SDK
+        "ft:gpt-5.5:my-org:custom:id",  # user-defined fine-tune, never in any SDK set
+    ],
+)
+async def test_openai_responses_system_prompt_uses_developer_role(model_name: str):
+    with respx.mock(base_url="https://api.openai.com") as mock:
+        mock.post("/v1/responses").mock(return_value=Response(200, json=make_response()))
+        provider = OpenAIResponses(model=model_name, api_key="test-key", stream=False)
+        body = await capture_request(
+            mock,
+            provider,
+            "You are a helpful assistant.",
+            [],
+            [Message(role="user", content="Hi")],
+        )
+
+    assert body["input"][0] == {
+        "role": "developer",
+        "content": "You are a helpful assistant.",
+    }
+    assert all(item.get("role") != "system" for item in body["input"])
+
+
+async def test_openai_responses_history_system_message_becomes_developer():
+    """Mid-session model switch via `_carry_context_to_session` seeds a
+    role='system' summary into the new session's history. With an OpenAI Responses
+    target (e.g. gpt-5.5), that history item must be re-mapped to role='developer'
+    on the wire — otherwise the first request after the switch is rejected.
+    """
+    carried_summary = Message(
+        role="system",
+        content="Summary carried from the previous model session: user asked about X.",
+    )
+    with respx.mock(base_url="https://api.openai.com") as mock:
+        mock.post("/v1/responses").mock(return_value=Response(200, json=make_response()))
+        provider = OpenAIResponses(model="gpt-5.5", api_key="test-key", stream=False)
+        body = await capture_request(
+            mock,
+            provider,
+            "You are a helpful assistant.",
+            [],
+            [carried_summary, Message(role="user", content="continue")],
+        )
+
+    roles = [item.get("role") for item in body["input"]]
+    assert "system" not in roles, body["input"]
+    assert roles[0] == "developer"  # system_prompt
+    assert roles[1] == "developer"  # carried-over summary
+    assert roles[2] == "user"

src/pythinker_code/agents/default/agent.yaml

@@ -7,6 +7,7 @@ agent:
   tools:
     - "pythinker_code.tools.agent:Agent"
     - "pythinker_code.tools.agent:RunAgents"
+    - "pythinker_code.tools.agent:ImplementAndJudge"
     - "pythinker_code.tools.skill:ReadSkill"
     # - "pythinker_code.tools.dmail:SendDMail"
     # - "pythinker_code.tools.think:Think"

src/pythinker_code/agents/default/judge.yaml

@@ -18,6 +18,7 @@ agent:
 
       ## Context Gate
       - Require the parent's packet: the original request, the diff or changed files, the commands actually run with their results, residual risks, and the draft final answer. If a load-bearing piece is missing, verdict BLOCKED and name it.
+      - When judging a code-change summary produced by `implementer`, require the implementer's `<coding_artifact>` block in the packet; if it is missing for a non-trivial code change, verdict BLOCKED and name it.
 
       ## External Claims (offline gate)
       You run offline by design — the verify profile blocks network and doc-lookup tools, so you never go online. External claims are judged by the parent's evidence, never by your own research or training-cutoff memory:
@@ -39,6 +40,7 @@ agent:
       - Safety and scope: no unsafe or destructive action, no secret or PII exposure, no scope creep beyond the request.
       - Production guardrails: changed code that touches caches, resources, trust boundaries, shared state, outbound requests, long-lived listeners, or authorization context has explicit defenses for stampedes, cleanup, schemas, races, retry storms, leaks, and IDOR risks.
       - Findings quality: for reports, each finding is actionable, anchored to evidence, and severity-ranked consistently with the base prompt's severity rubric (critical/high/medium/low/info).
+      - Minimum-diff: the diff takes the smallest rung of the reduction ladder (skip-need → reuse-stdlib → use-native → use-installed-dep → one-line → minimum) before adding new code; no abstractions, dependencies, config keys, files, or error paths that the brief did not ask for. New dependencies require a one-line justification; new config keys require a one-line consumer. The ladder applies uniformly — judge once, not per-mode.
 
       ## Role Exit Checklist
       - PASS: sound; at most minor wording nits remain.

src/pythinker_code/agents/default/system.md

@@ -97,6 +97,8 @@ Build only after requirements are understood (ask if unclear) and evidence is ga
 
 For refactors, update every call site the interface change touches, and do not alter existing logic — especially in tests — beyond what the change requires. For features, add tests if the project already has tests. Migrations go additive before destructive, reversible where the framework allows; never edit a migration that already shipped. Identify the synchronization model in use and conform to it; explicitly flag any new lock, atomic, or async-boundary change. Update comments, docstrings, and README snippets your change makes false — stale documentation is a bug you just wrote.
 
+**Default to `ImplementAndJudge` for non-trivial scoped edits** instead of calling `implementer` and `judge` separately. The chain runs `implementer` first, hands the artifact to `judge`, and on `NEEDS_WORK` re-invokes `implementer` once with the judge's `REQUIRED FIXES` under a `## Revision brief` section before re-judging. The implementer cap is two invocations total — a still-`NEEDS_WORK` after revision surfaces the contradiction to you and stops. Pass `scope` so the judge can `git diff` only the allowed paths; pass `acceptance` to give the judge concrete pass conditions beyond the rubric.
+
 ### 4.5 Research & file generation
 
 For research or multimedia tasks (images, video, PDFs, docs, spreadsheets, presentations): clarify requirements first, plan before deep or wide research, design search queries deliberately. Detect tools already in the environment before installing anything; third-party installs go in an isolated/virtual environment. After generating or editing any media file, read it back to confirm the content. Never install to or delete from outside the working directory without confirmation.
@@ -121,6 +123,8 @@ For research or multimedia tasks (images, video, PDFs, docs, spreadsheets, prese
 
 **Judge gate.** Before delivering high-stakes or hard-to-reverse work, run an independent `judge` subagent as the last step when available. Triggers — any one suffices: a change spanning multiple files or touching production guardrail surfaces (§6); a deliverable the user will merge, deploy, publish, or act on; a security audit or any severity-scored findings report; a release or destructive action. When unsure whether work is high-stakes, treat it as high-stakes; skip it for low-stakes, reversible, or trivial work. Hand the judge a tight packet: original request, the diff or changed files, the commands actually run with their results, residual risks, and your draft answer. It is one cheap spot-checking pass that gates your evidence — it does not redo work or replace deterministic tests and lint, so run those first. Treat `NEEDS_WORK` or `BLOCKED` as a stop: fix or revise, re-judge only if the change was material. When the judge is unavailable, walk the same checklist yourself, lead with the same `PASS`/`NEEDS_WORK`/`BLOCKED` verdict, state what verification actually ran, and put any missing packet element under **BLOCKERS**.
 
+Default to `ImplementAndJudge` for non-trivial scoped edits (see §4.4); reserve a bare `judge` call for non-implementation reviews — reports, audits, severity-scored findings, or answers that don't ship code.
+
 **Background shell** (root agent only). Launch long-running commands via `Shell` with `run_in_background=true` and a short `description`; the system notifies you at terminal states. `TaskList` re-enumerates active tasks (especially after context compaction); `TaskOutput` gives non-blocking snapshots (`block=true` only to intentionally wait); `TaskStop` cancels. After starting a background task, default to returning control to the user. The only task-management slash command for users is `/task` — never invent subcommands like `/task list` or `/tasks`. Subagents and sessions without these tools must not assume background-task control.
 
 **Skills (`ReadSkill`).** Load a skill's exact instructions before applying its workflow — mandatory for `review-pr`, `diagnose-ci-failures`, `fix-errors`, `implement-specs`, `spec-driven-implementation`, `check-impl-against-spec`, `resolve-merge-conflicts`, and `create-pr`. Read skill details only when needed, to conserve context. Catalog and scope precedence in §12.