fix: one review rubric, and a session retry that knows why it failed (#166, #167)

[thedancingdeveloper]

Two items were rejected today for artefacts already found, fixed and closed —
in the other copy of the prompt. Found running session mode against a second
repository (#56).

#167 — the rubric was duplicated

executor.py and session_executor.py each carried a REVIEW_PROMPT. Every
correction made from measurement landed in the headless one:

Fix	headless	session
a diff is the whole change (#159)	yes	no
the harness ran the checks, and which (#159)	yes	no
the files the diff touched (#156)	yes	no
scope belongs to the task (#159)	yes	no

So R4 added a paragraph to a diagnostics panel and was rejected on:

the diff does not demonstrate that it is specifically integrated into the
existing diagnostics/support surface rather than merely adjacent

That is precisely what #156 documented and closed — a diff cannot show its
surroundings, and the fix was to supply them. R3's rejection carried the same
shape.

Both rejections also contained a real point — R3 widened a repository
trait's return type, R4's wording claimed everything absent from the bundle was
deliberate. The reviews were not wrong. Half of each objection was a solved
problem in the wrong file.

The rubric is now imported rather than copied, and review_context /
review_checks_prompt are module-level functions both executors call.

Two tests guard it: the prompts must be the same object, because "roughly
the same" is what lets drift back; and _review must actually pass the fields,
because identical prompts with empty fields would keep the reviewers unequal
while passing the first test.

#166 — a session retry was blind

The agent's prompt carried the title, the brief and the checks. Not the
reviewer's verdict, not the last failure. R3's retry would have been sent the
identical brief with no mention of the specific criticism it had just received.

A session attempt is minutes of a real agent rather than one API call, so
repeating one blind is the most expensive avoidable thing the harness does.

Not resumption: the item is re-read from the current brief, no prior diff
is fed back, and a guided attempt consumes an attempt exactly as an unguided
one does. A first attempt reads exactly as before — asserted.

One thing the tests did not catch

The first version of this borrowed Executor._review_context as an unbound
method on a SessionExecutor, which has no context_policy and would have
raised at runtime. The suite passed anyway, because the test exercising that
path lives on another branch. Extracting real shared functions removes the
class of mistake rather than the instance.

All four gates green.

🤖 Generated with Claude Code