Skip to content

refactor(*): close out the participant contract the five plugins now write - #495

Merged
ypflll merged 8 commits into
mainfrom
refactor/participant_contract_closeout
Sep 20, 2026
Merged

ypflll merged 8 commits into
mainfrom
refactor/participant_contract_closeout

Conversation

@ypflll

@ypflll ypflll commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

#469 gave a bundled sub-agent nine verbs in place of six hook phases. Reviewing it turned up a
list of things the contract said but did not yet do, and this PR closes that list. No behaviour
change is intended: the point is that the same seam becomes something a role can replace and
something a generated judgement can enter.

1. The roles are named apart from the verbs, and observe is gone. ActionModule.review
and the participant's review were two different things a line apart. The four delegating role
methods now say what they do -- they ask -- and the verbs keep their own names, so ask_review
asks review. observe is removed: a verb with no return value decides nothing and can only
keep state a participant is forbidden to keep. outbound says on its own docstring that it is
the one verb with no role seat, and StepView gains phase, so a verb asked at two moments
reads which one it is rather than inferring it from whichever other field happens to be set
(tools_ran becomes a property derived from it, so the two cannot disagree).

2. The three verbs the seat was applying itself get a seat. system_addendum, archive and
select_tools were rendered straight onto the hook decision, so each was the one judgement in
the set with nowhere to compose two participants, nowhere to vet what came back, and nothing a
replacement role could decide. They join the other four:
MemoryModule.ask_system_addendum / ask_archive and CapabilityModule.ask_select_tools. The
seat is not the point where narrowing takes effect -- select still is, and the registry still
adjudicates every call -- so a participant narrows after the product has spoken and never instead
of it.

3. A participant answers with plain data. review, intake and system_addendum returned
host classes (Verdict, Intake) and salvage returned Any. Accept, Resample, End and
Intake are now builders returning plain mappings, the host reads one back with read_verdict /
read_intake, and an answer it cannot read is silence rather than a refusal. That is what makes
the third invariant real and what lets a judgement compiled from source answer the same verbs a
plugin does -- it can build a dict; it cannot build a host class.

4. A dispatch's judgements are a participant. judge was the one verb a role answered by
reading the dispatch's own checks and code for itself. A Charter now enters as a participant
like any other, ActionModule.ask_judge takes the list, and the merge is a veto so the first
refusal decides. The participant list becomes the only route an external judgement takes, which
is the thing step four needs: a judgement generated for one dispatch joins the same list without
a second way in.

5. The rename. AgentConduct is AgentParticipant, contracts/agent_conduct.py is
contracts/participant.py, agent/hook/conduct.py is agent/hook/participant.py, and
agent/harness/conducts.py is agent/harness/participants.py. "Conduct" was already taken in
this tree -- raven-code's Coding Conduct prompts are a different thing -- and a reader hitting both
had no way to tell them apart. The rename was done symbol by symbol, not by search and replace,
for exactly that reason.

7. Review (LivXue, at 3a1bf07). The judge disclosure had reached two of its four homes;
the other two are ActionModule.ask_judge on the paper and DefaultAction.ask_judge, and they
survived because they phrase the claim without naming any symbol the earlier fix touched. The test
meant to pin that claim could not fail for the reason it was written -- it asserted the refusal
rather than the argument, and built a ParticipantHook it dropped -- and now asserts what the
registry hands the role, mutation-checked by wiring a participant through. compose_judge
exploded a one-sentence str refusal into one refusal per character, because str satisfies
Sequence[str] structurally; charter_code.run_judge coerced it where this composition used to
live and the handling did not move with it. Two records corrected: the type-only
raven.contracts -> raven.agent edge, the first in the tree and the price of moving Verdict
and Intake off the paper, is now written beside the import contract it bends rather than left to
be discovered; and the ledger entry for the 3,200 -> 3,410 bump is restored to the names and count
it was written with. Not taken: reading step.phase at research_flow/flow.py:1000 -- the gate
chain's phase vocabulary and StepView.phase coincide on after_iteration only, and that call
site wants before_execute_tools, which is the key _phase dispatches on.

6. What the paper was claiming and not doing. Read back over the seam afterwards rather than
over the diff. judge read as though a plugin's override were asked ahead of a dispatch's own
rules; it is not, and cannot be as written, because the party that asks the verb is
ToolRegistry.execute, which is not the hook chain and holds no handle on this turn's
participants. The merge order is real; nothing puts a plugin in the list. Wiring one through is a
turn-scoped register this change does not add, and it would start refusing calls that run today,
so it is named on the paper and pinned by a test instead. max_iterations had lost its
docstring to the property inserted between the field and the string documenting it. No executable
line moves in this commit: the module's AST, docstrings stripped, is identical to its parent's.

CONTEXT.md's entry is rewritten to the end state, including the two kinds of role method, the two
verbs that are exceptions, and the note that the Coding Conduct prompts are unrelated.

One item from the same list is deliberately not here: a planning.* block on the dispatch
parameters. It is not a contract closeout but a new model-facing field, and it needs the
generator's schema and prompt to move with it.

Contract papers: participant.py and harness.py are both factory-loop tier, so the
contract-tier digest and CONTRACTS_VERSION do not move. CONTRACTS_LINE_CEILING 3,440 to
3,470 to 3,500 to 3,540 to 3,550 to 3,570, each bump with its own review paragraph in
tests/test_kernel_budget.py. The chain was recomputed after rebasing onto #410, which added
seven lines to loop_hooks.py inside the old headroom and left one commit here with one line of
it; every "Measured at" figure in that file is the count at that commit.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

Gates, run at each of the seven commits:

uv run ruff check --no-cache raven evolver agents plugins-dist tests scripts
uv run ruff format --check --no-cache raven evolver agents plugins-dist tests scripts
uv run lint-imports
uv run pytest -q
npx commitlint --from origin/main --to HEAD --config commitlint.config.cjs
uv run python scripts/check_commit_messages.py origin/main..HEAD
uv run python scripts/check_source_language.py origin/main..HEAD
uv run python scripts/check_large_files.py origin/main..HEAD

Import contracts: 10 kept, 0 broken. The suite is green: the failures this machine shows are the
same ones it shows on an unmodified main in the same worktree layout (missing browser and
optional packages), and this branch adds none.

Behaviour preservation. The oracle is the hook seam itself. main and this branch were run under
the same scripted provider, outputs normalised for nonces and temp paths, then byte-compared.
Four scenarios, all identical:

  • the five-plugin hook seam, every phase's decision and metadata per iteration (23,840 bytes)
  • a whole turn with no playbook: host turn, spawned sub-agent, tool calls and refusals
  • a playbook-on worker lane in process, including the charter refusals
  • six window-shrink scenarios

This is the seam the change is about, and identical output across it is what "no behaviour
change" means here -- it is not a claim about paths the four scenarios do not reach.

Two claims had no assertion anywhere and now do. phase, and the tools_ran derived from
it, are what three of the five bundled plugins branch on -- ppt on its close, oncall on its
counter, research on which gate runs -- and a mistyped phase string at one of the six call sites
would have changed what they do with a green suite. The read-only test now also covers
mode_overlay, the one frozen field it skipped. All three assertions were mutation-checked:
mislabelling a phase, deriving tools_ran from the wrong one, and dropping the freeze each
turn exactly their own test red.

New tests, each written against a claim the contract now makes rather than against the code:
a participant cannot write through the StepView it is shown; an answer the host cannot read is
silence, not a refusal; a participant that answers with a mapping it built itself is understood;
every verb with a seat goes through its role rather than around it, asserted as a set so a new
verb that bypasses its role fails; two seats in one chain each keep one addendum; two turns in
flight keep their own participants; a bound role decides what a participant's verdict does; a
plugin's own judgement speaks before the dispatch's, and a silent participant lets the next one
speak.

Real model, Sonnet 4.5 through OpenRouter, on this branch: a query generates a playbook, two
sub-agents are instantiated and run, each writes its summary.md under its own fence, and both
results are announced back to the originating session.

  • Relevant tests pass locally
  • Relevant lint / type checks pass locally
  • User-facing docs or screenshots are updated when needed

Risk

For plugin authors: the six-phase AgentHook is untouched and a plugin shipping one keeps
working. A plugin written against #469's AgentConduct needs the new names and, if it answered
with Verdict or Intake, the builders instead -- the five bundled plugins are ported here and
are the worked examples.

One verb narrowed, and silently for a plugin that used the width. salvage was annotated
Any | None and compose_salvage returned whatever was not None; it is now str | None and only
a non-empty string is taken. A third-party plugin that salvages with a reply object rather than
text stops being heard rather than raising -- which is the third invariant working as written, and
worth knowing before upgrading. The narrowing is deliberate: salvage answers the text a turn
sends when it produced none, and Any was not a contract.

For anyone replacing a harness role: the four delegating methods are renamed and four more are
added, so a replacement written against #469 needs them. bind() names the missing member at
assembly rather than failing inside a turn.

Rollback is a revert of the merge commit. Nothing persisted on disk changes shape.

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

Related Issues

Follows #468 and #469.

Generated with Claude Code (https://claude.com/claude-code)

@ypflll
ypflll requested review from 0xKT and LivXue September 18, 2026 07:20

@LivXue LivXue left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 3a1bf07. Read the whole diff rather than a slice.

I started at ec9cccc and re-scoped when 3a1bf07 landed mid-review. That commit closed three of
what I had: the orphaned max_iterations docstring, the absence of any assertion on phase /
tools_ran, and the judge claim -- the last by disclosure rather than by wiring, which I think
is the right call and is argued well. test_each_phase_names_itself_on_the_step is a good shape:
the whole ordered list of six phases with their derived booleans, so a mistyped literal at any of
the six call sites turns exactly one test red. Everything below is against the current head only.

Method. "Reproduced" means I ran the branch's own code and pasted what came back; everything
else is code reading and says so. I measured the raven/contracts line count at every commit
rather than reading the figures. Lenses covered: rename completeness across the whole tracked
tree; the seat's phase-by-phase equivalence with the base; the merge rules base to head; the
import contracts and what the config excludes from them; every surface that states what judge
composes; test non-vacuity; and Windows.

Two please-fix-before-merge, both because the judge disclosure has not reached everywhere it
needs to:

  • contracts/harness.py:351 and agent/harness/action.py:46 still carry the wide claim. Two of
    the four homes were corrected in 3a1bf07; these are the other two, and they are the ones a
    reader of the role protocol meets first.
  • tests/test_participant_hook.py:340 cannot fail for the reason it was written. It collects
    asked and asserts only refusals, so the wiring it exists to catch would leave it green.

Four non-blocking: the new raven.contracts -> raven.agent edge that
exclude_type_checking_imports hides from lint-imports; compose_judge exploding a str refusal
into characters (reproduced); CONTEXT.md:174 saying "a list of names" where the paper argues
definitions; and the half-updated ledger paragraph.

One with no line to hang it on. The Risk section does not mention that salvage narrowed.
compose_salvage now keeps only a non-empty str; the base returned any non-None object and
AgentConduct.salvage was annotated Any | None. A third-party plugin that salvages with a reply
object goes silent rather than erroring. The Summary mentions the change in passing, but Risk is
where a plugin author looks, and squash makes this description the commit body on main -- so the
commit body saying it is not enough.

Merge state, as of this comment. The branch is dirty. #410 landed on main after this branch
was cut, and both touch agents/raven-research/plugins/research-flow/research_flow/flow.py -- the
only overlapping file. The green CI at this head is therefore not evidence about the merged tree.
While that file is open for the rebase: line 983 reconstructs "after_iteration" from
step.tools_ran, which is now itself derived from step.phase. The string it wants is in hand.

Checked and deliberately not filed:

  • Rename completeness. Zero hits for AgentConduct, ConductHook, ConductFactory,
    agent_conduct, harness.conducts anywhere in the tracked tree, markdown and prompts included.
    raven-code's own conduct vocabulary is correctly untouched and CONTEXT.md carries the
    disambiguation. Doing this symbol by symbol was the right call and it shows.
  • observe removal. No implementation anywhere; the remaining observe( hits are an unrelated
    saturation counter and a browser MutationObserver.
  • tools_ran semantics. Checked phase by phase against the base: only after_iteration is True,
    as before.
  • compose_review. Equivalent to the base's replace(accepted[0], note=...), including the
    case where the first participant said nothing.
  • The empty-tool-array case. compose_tools returning [] survives the seat's
    narrowed is not None check, so narrowing to nothing is not swallowed by a truthiness test.
    Easy to get wrong; not gotten wrong here.
  • Fault tolerance is genuinely better, and I do not think the description claims enough credit for
    it. The base's review returned Verdict and compose_review read .accepted off it, so a
    conduct answering None raised into the composite's catch and lost the phase. read_verdict
    makes that an accept.
  • Windows. The diff introduces no filesystem, path, newline, encoding or subprocess surface;
    Windows self-upgrade is green at this head.
  • CI. Green at this head. The one failing pull request title row is a superseded run from
    07:09:30, passing at 07:11:29.

Filed as a comment rather than changes-requested: nothing here is wrong at runtime today, and the
two please-fix items are two sentences and one assertion.

Comment thread raven/contracts/harness.py Resolved
Comment thread raven/agent/harness/action.py Resolved
Comment thread tests/test_participant_hook.py Resolved
Comment thread raven/contracts/harness.py Resolved
Comment thread raven/agent/harness/participants.py Resolved
Comment thread CONTEXT.md Resolved
Comment thread tests/test_kernel_budget.py Resolved
ypflll pushed a commit that referenced this pull request Sep 18, 2026
… becoming eighteen

Review on #495 (LivXue), at 3a1bf07. Five of its findings; two it raised are
answered rather than taken, below.

The judge disclosure had reached two of its four homes. The two it had not are
``ActionModule.ask_judge`` on the paper and ``DefaultAction.ask_judge``, and
those are the ones a reader of the role protocol meets first. They survived
because they phrase the claim without naming anything the earlier fix touched:
"composed from each participant's judge", "asked after whatever the caller
passes". There is one caller and it passes nothing. Both now say so.

The test meant to pin that could not fail for the reason it was written. It
collected what the participant was asked and then asserted only that the
Charter still refused -- true before this PR and true after any wiring anyone
might add -- and it built a ``ParticipantHook`` it then dropped, so the factory
never ran and the collection could not have been non-empty whatever the
production code did. It now asserts the argument the registry hands the role,
which is the fact the three docstrings rest on. Mutation-checked by wiring a
participant through ``_verifier_refusals``: the old assertion stays green, the
new one turns red.

``compose_judge`` exploded a one-sentence refusal into one refusal per
character: a bare ``str`` satisfies ``Sequence[str]`` structurally, so nothing
in the annotation or a type checker objects. ``charter_code.run_judge`` coerced
it where this composition used to live and moving the composition out a layer
did not bring the handling along. Unreachable today -- the Charter path is
normalised before it arrives and no bundled plugin implements the verb -- but
this is the seam meant to carry generated judgements, and a bare string is a
plausible thing for one to answer. Coerced, with a test, mutation-checked.

Two records corrected. The type-only ``raven.contracts`` -> ``raven.agent``
edge, the first in the tree, is the price of moving ``Verdict`` and ``Intake``
off the paper; it costs nothing at import time and lint-imports does not see it
through ``exclude_type_checking_imports``, so it is now written beside the
contract it bends rather than left to be discovered. And the ledger entry for
the 3,200 -> 3,410 bump had been renamed but not refreshed, so it read as a
present-tense description of a file whose line count and exported names had
both moved; it is restored to the names it was written with, with a line saying
an entry is a dated record and not a description of today.

CONTEXT.md said a verb answers with "a list of names". None does --
``select_tools`` answers with definitions, which the paper spends a paragraph
insisting on.

Not taken, with reasons. The suggestion to read ``step.phase`` at
research_flow/flow.py:1000 instead of reconstructing from ``step.tools_ran``:
the two vocabularies coincide on ``after_iteration`` only. That call site wants
``before_execute_tools``, which is the gate chain's own phase name and the key
``_phase`` dispatches on; ``StepView.phase`` says ``execute_tools``.
Substituting would route the call to no gate. The salvage narrowing belongs in
the PR description's Risk section and has been added there, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ypflll
ypflll force-pushed the refactor/participant_contract_closeout branch from 3a1bf07 to c3524fc Compare September 18, 2026 08:08
@ypflll

ypflll commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Thanks -- this was a good read, and the two it caught that I had just written myself are the ones I
most needed caught. Pushed as c3524fce, on top of a rebase onto #410. Point by point.

Taken, all three please-fix.

ActionModule.ask_judge and DefaultAction.ask_judge now say what is wired. Your note on why
they survived is the part I want to keep: both phrase the claim without naming any symbol the
earlier fix touched, so grepping judge found the fix sites and not these. I had grepped the
symbol. Grepping the claim is the habit.

test_a_seated_participant_is_not_asked_to_judge_a_tool_call is gone and replaced by
test_the_registry_asks_judge_with_no_participants_at_all, which spies on the role and asserts
handed == [()]. You are right about both halves -- asked was collected and discarded, and the
ParticipantHook was constructed and dropped so the factory never ran and it could not have been
non-empty regardless. I mutation-checked the original but only against the phase literals, never
against the wiring, which is exactly the axis the test existed for. The replacement is checked the
right way: wiring a participant into _verifier_refusals leaves the old assertion green and turns
the new one red.

compose_judge coerces a bare str now, the way run_judge does, with a test and a mutation
check. Reproduced your eighteen before fixing. Your framing is the right one -- the composition
moved out a layer and the handling did not move with it.

Taken, the four non-blocking.

The raven.contracts -> raven.agent type-only edge is written down in two places: beside the
"kernel stands alone" contract in pyproject.toml, and at the import itself. You asked whether it
is the intended price or whether the shapes belong somewhere both sides can name. It is intended
-- a host class on the paper is the thing a generated judgement cannot build, which is the whole
of commit 3 -- but "intended and undocumented" was not a defensible state for the first edge in
the tree, particularly with that comment asserting no exceptions. If the papers ever ship as their
own wheel, that comment now names the one edge to resolve.

CONTEXT.md said "a list of names"; it says definitions now. You are right that it is the worst
possible place for that error, given the paper argues the distinction hardest.

The 3,200 -> 3,410 ledger entry is restored to the names and the count it was written with, plus a
line saying an entry is a dated record of one bump rather than a description of today's file. Your
diagnosis of "half of each" is what made the right fix obvious.

Your reading of the three commit bodies' ceiling arithmetic was correct, and the rebase onto #410
made it moot in an instructive way: #410 added seven lines to loop_hooks.py inside the old
headroom, so every measurement in this branch moved and one commit was left with a single line of
room. The whole chain is recomputed -- 3,470 / 3,500 / 3,540 / 3,550 / 3,570 -- and the prose
figures with it.

Not taken, one.

research_flow/flow.py:1000. The string is not in hand. That call site wants
"before_execute_tools", which is the gate chain's own phase vocabulary and the key _phase
dispatches on; StepView.phase says "execute_tools". The two vocabularies coincide on
"after_iteration" and differ everywhere else -- the plugin kept the six hook-phase names
deliberately so six thousand lines of gate bodies did not have to move. Substituting would route
that call to no gate. I have left it and said so in the commit body.

The one with no line to hang it on: taken, and you are right about where it belongs. salvage
narrowed from Any | None to str | None, and compose_salvage from "any non-None" to "a
non-empty str", so a third-party plugin salvaging with a reply object goes quiet rather than
erroring. That is now a paragraph in the Risk section, which is where you said a plugin author
looks, and it survives squash.

On the merge state. Rebased onto 210e2971. The only overlapping file was the one you named,
and both conflicts in it were the same import block -- #410's TURN_ASK_KIND_KEY import against
my rename -- resolved by keeping both rather than either. Re-verified on the merged tree: the
five-plugin seam A/B against the new main is byte-identical (23,952 both sides), the full suite's
failure set is a strict subset of new main's (127 vs 127 failed, 88 vs 88 errors, zero branch-only
failures), and the contract gates pass at each of the seven commits individually.

ypflll pushed a commit that referenced this pull request Sep 18, 2026
… becoming eighteen

Review on #495 (LivXue), at 3a1bf07. Five of its findings; two it raised are
answered rather than taken, below.

The judge disclosure had reached two of its four homes. The two it had not are
``ActionModule.ask_judge`` on the paper and ``DefaultAction.ask_judge``, and
those are the ones a reader of the role protocol meets first. They survived
because they phrase the claim without naming anything the earlier fix touched:
"composed from each participant's judge", "asked after whatever the caller
passes". There is one caller and it passes nothing. Both now say so.

The test meant to pin that could not fail for the reason it was written. It
collected what the participant was asked and then asserted only that the
Charter still refused -- true before this PR and true after any wiring anyone
might add -- and it built a ``ParticipantHook`` it then dropped, so the factory
never ran and the collection could not have been non-empty whatever the
production code did. It now asserts the argument the registry hands the role,
which is the fact the three docstrings rest on. Mutation-checked by wiring a
participant through ``_verifier_refusals``: the old assertion stays green, the
new one turns red.

``compose_judge`` exploded a one-sentence refusal into one refusal per
character: a bare ``str`` satisfies ``Sequence[str]`` structurally, so nothing
in the annotation or a type checker objects. ``charter_code.run_judge`` coerced
it where this composition used to live and moving the composition out a layer
did not bring the handling along. Unreachable today -- the Charter path is
normalised before it arrives and no bundled plugin implements the verb -- but
this is the seam meant to carry generated judgements, and a bare string is a
plausible thing for one to answer. Coerced, with a test, mutation-checked.

Two records corrected. The type-only ``raven.contracts`` -> ``raven.agent``
edge, the first in the tree, is the price of moving ``Verdict`` and ``Intake``
off the paper; it costs nothing at import time and lint-imports does not see it
through ``exclude_type_checking_imports``, so it is now written beside the
contract it bends rather than left to be discovered. And the ledger entry for
the 3,200 -> 3,410 bump had been renamed but not refreshed, so it read as a
present-tense description of a file whose line count and exported names had
both moved; it is restored to the names it was written with, with a line saying
an entry is a dated record and not a description of today.

CONTEXT.md said a verb answers with "a list of names". None does --
``select_tools`` answers with definitions, which the paper spends a paragraph
insisting on.

Not taken, with reasons. The suggestion to read ``step.phase`` at
research_flow/flow.py:1000 instead of reconstructing from ``step.tools_ran``:
the two vocabularies coincide on ``after_iteration`` only. That call site wants
``before_execute_tools``, which is the gate chain's own phase name and the key
``_phase`` dispatches on; ``StepView.phase`` says ``execute_tools``.
Substituting would route the call to no gate. The salvage narrowing belongs in
the PR description's Risk section and has been added there, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ypflll
ypflll force-pushed the refactor/participant_contract_closeout branch from c3524fc to 6d82c45 Compare September 18, 2026 08:16
@ypflll
ypflll requested a review from LivXue September 18, 2026 08:22
@LivXue

LivXue commented Sep 18, 2026

Copy link
Copy Markdown
Member

Re-reviewed at 6d82c455. Every fix verified, with the method in each of the seven threads, all now replied to and resolved. Two things belong here rather than on a line.

You were right and I was wrong about research_flow/flow.py. I checked the reason rather than taking it, and it holds. _phase dispatches on the gate chain's own keys, and the two vocabularies line up like this:

gate chain (_phase keys)   StepView.phase
before_user_inbound        user_inbound
before_iteration           iteration
before_execute_tools       execute_tools
after_iteration            after_iteration    <- the only member they share
terminal_answerless        answerless
after_send                 sent

step.phase is "execute_tools" at the moment that line needs "before_execute_tools", so the substitution would dispatch to a key with no gate behind it.

Naming what misled me, since that is the reusable part: I generalised from the one member where the two vocabularies coincide. That line reads "after_iteration" on both sides of its conditional, which made the string look already in hand, and I never checked the other five keys against the dispatch table. Two phase vocabularies overlapping on exactly one member is a nasty shape -- the shared member is the one a reader samples, and it makes the two look like one vocabulary. Your commit body is the right home for that.

One bookkeeping note. Your reply cites c3524fce. That sha is not on the branch: it is the pre-rebase commit with the same subject, and git merge-base --is-ancestor c3524fce HEAD says no. The commit carrying this work is 6d82c455. Worth mapping, since a rebase stales every sha in a review trail and this one still resolves to an orphan object for now.

State as I read it. Rebase clean -- 210e2971 is an ancestor, 0/7 left-right, and the only overlapping file is the one you named, with both imports kept rather than either. 14 checks green at this head, two unit shards still running. mergeable_state is blocked rather than dirty, so the conflict is gone and what remains is the approval gate.

No further findings. I have not independently verified the two claims that need your harness -- the seam A/B at 23,952 bytes on the merged tree, and the failure-set subset at 127/127 and 88/88 -- and I am taking those as reported.

@ypflll

ypflll commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the second pass -- all seven threads verified and resolved on your side, nothing new
filed, and the correction on your own comment's re-anchoring was a courtesy to whoever reads the
thread later.

State as it stands: seven commits, rebased onto 027637b0 (main moved twice more; neither commit
touched a file this branch touches), 17/17 checks green at 6d82c455, zero unresolved threads.
The suite's failure set on this machine is identical to main's, and the five-plugin seam A/B is
byte-identical against the current base.

reviewDecision is still REVIEW_REQUIRED -- both your rounds were filed as COMMENTED, which
matches what you said the first time about nothing being wrong at runtime. If you are satisfied
after the second pass, an approval is the one thing left to unblock it. If you would rather take
another look first, or you want the roster change discussed below landed before this merges, say
so and I will hold.

For context on what comes next, since it touches this seam: the roles are still asked once per
seat with a roster of one, so the merge rules in harness/participants.py are idle for every verb
but judge. That is a behaviour change for a chain with two or more seats, so it is its own PR
rather than more commits here.

@ypflll

ypflll commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor Author

Correction, and thank you for catching it. c3524fce is not on the branch -- git merge-base --is-ancestor c3524fce HEAD says no here too. It is the pre-rebase commit with that subject, and
citing it in a review trail that outlived two rebases was sloppy of me.

The trail spans three bases now, so here is the whole map rather than the one sha. Subjects are
identical down each row; only the base moved.

first review    #410 rebase   027637b0 rebase   subject
ec9cccc7    ->  b3f30d62  ->  f2ca709f          ask_* on the asking half
3a1bf07c    ->  0ec2c9da  ->  0f0da098          say on the paper the two things it was not doing
c3524fce    ->  --------  ->  9120476f          finish the judge disclosure

git diff c3524fce 9120476f is exactly main's two intervening commits
(context_engine/segments/render.py and its tests, plus the benchmark chart) and nothing of mine,
so every verification either of us did at c3524fce still stands at 9120476f -- but the object
is only reachable while a reflog holds it, which is the part that matters for anyone reading this
later. From here I will cite the head sha and say what it was rebased from.

On research_flow: the table you built is the clearer artefact, and "two phase vocabularies
overlapping on exactly one member" is the sentence worth keeping -- the shared member is the one a
reader samples. I have added your table to the commit body so it travels with the code rather than
living only in this thread. That amend is message-only -- git diff 6d82c455 9120476f is empty --
and it is why the head is 9120476f rather than the 6d82c455 you verified; the tree you read is
the tree that is there.

Noted that the seam A/B and the failure-set subset are taken as reported. Both are scripted and
the scripts are reproducible off the branch; say the word if you want them attached or re-run
against anything in particular.

ypflll pushed a commit that referenced this pull request Sep 18, 2026
… becoming eighteen

Review on #495 (LivXue), at 3a1bf07. Five of its findings; two it raised are
answered rather than taken, below.

The judge disclosure had reached two of its four homes. The two it had not are
``ActionModule.ask_judge`` on the paper and ``DefaultAction.ask_judge``, and
those are the ones a reader of the role protocol meets first. They survived
because they phrase the claim without naming anything the earlier fix touched:
"composed from each participant's judge", "asked after whatever the caller
passes". There is one caller and it passes nothing. Both now say so.

The test meant to pin that could not fail for the reason it was written. It
collected what the participant was asked and then asserted only that the
Charter still refused -- true before this PR and true after any wiring anyone
might add -- and it built a ``ParticipantHook`` it then dropped, so the factory
never ran and the collection could not have been non-empty whatever the
production code did. It now asserts the argument the registry hands the role,
which is the fact the three docstrings rest on. Mutation-checked by wiring a
participant through ``_verifier_refusals``: the old assertion stays green, the
new one turns red.

``compose_judge`` exploded a one-sentence refusal into one refusal per
character: a bare ``str`` satisfies ``Sequence[str]`` structurally, so nothing
in the annotation or a type checker objects. ``charter_code.run_judge`` coerced
it where this composition used to live and moving the composition out a layer
did not bring the handling along. Unreachable today -- the Charter path is
normalised before it arrives and no bundled plugin implements the verb -- but
this is the seam meant to carry generated judgements, and a bare string is a
plausible thing for one to answer. Coerced, with a test, mutation-checked.

Two records corrected. The type-only ``raven.contracts`` -> ``raven.agent``
edge, the first in the tree, is the price of moving ``Verdict`` and ``Intake``
off the paper; it costs nothing at import time and lint-imports does not see it
through ``exclude_type_checking_imports``, so it is now written beside the
contract it bends rather than left to be discovered. And the ledger entry for
the 3,200 -> 3,410 bump had been renamed but not refreshed, so it read as a
present-tense description of a file whose line count and exported names had
both moved; it is restored to the names it was written with, with a line saying
an entry is a dated record and not a description of today.

CONTEXT.md said a verb answers with "a list of names". None does --
``select_tools`` answers with definitions, which the paper spends a paragraph
insisting on.

Not taken, with reasons. The suggestion to read ``step.phase`` at
research_flow/flow.py:1000 instead of reconstructing from ``step.tools_ran``:
the two vocabularies coincide on ``after_iteration`` only.

    gate chain (_phase keys)   StepView.phase
    before_user_inbound        user_inbound
    before_iteration           iteration
    before_execute_tools       execute_tools
    after_iteration            after_iteration    <- the only member they share
    terminal_answerless        answerless
    after_send                 sent

That call site wants ``before_execute_tools``, the key ``_phase`` dispatches on,
where ``StepView.phase`` says ``execute_tools``, so substituting would route it
to a key with no gate behind it. The table is the reviewer's, built while
checking the reason rather than taking it. Two phase vocabularies overlapping on
exactly one member is a nasty shape: the shared member is the one a reader
samples, and it makes the two look like one vocabulary. The salvage narrowing belongs in
the PR description's Risk section and has been added there, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ypflll
ypflll force-pushed the refactor/participant_contract_closeout branch from 6d82c45 to 9120476 Compare September 18, 2026 08:32
@gloryfromca

Copy link
Copy Markdown
Member

No blockers; this can merge as far as I am concerned.

The correction is accurate. 9120476f has the same tree as the reviewed 6d82c455; the only delta is the final commit message, which now preserves the phase-vocabulary explanation and table. The seven review threads remain resolved.

Verified on the current head:

  • git diff --exit-code 6d82c4558bca36126e7deed393e032142531d3f7 9120476f8f2d850a1a38067d9b3071dfdbf1e70b -- . passed with no tree diff.
  • uv run pytest tests/test_participant_hook.py tests/test_kernel_budget.py -x: 25 passed.
  • gh pr checks 495 --repo EverMind-AI/Raven: all 17 checks passed.

The multi-seat roster behavior described for the follow-up is a separate behavior change and does not block this closeout.

ypflll pushed a commit that referenced this pull request Sep 18, 2026
… becoming eighteen

Review on #495 (LivXue), at 3a1bf07. Five of its findings; two it raised are
answered rather than taken, below.

The judge disclosure had reached two of its four homes. The two it had not are
``ActionModule.ask_judge`` on the paper and ``DefaultAction.ask_judge``, and
those are the ones a reader of the role protocol meets first. They survived
because they phrase the claim without naming anything the earlier fix touched:
"composed from each participant's judge", "asked after whatever the caller
passes". There is one caller and it passes nothing. Both now say so.

The test meant to pin that could not fail for the reason it was written. It
collected what the participant was asked and then asserted only that the
Charter still refused -- true before this PR and true after any wiring anyone
might add -- and it built a ``ParticipantHook`` it then dropped, so the factory
never ran and the collection could not have been non-empty whatever the
production code did. It now asserts the argument the registry hands the role,
which is the fact the three docstrings rest on. Mutation-checked by wiring a
participant through ``_verifier_refusals``: the old assertion stays green, the
new one turns red.

``compose_judge`` exploded a one-sentence refusal into one refusal per
character: a bare ``str`` satisfies ``Sequence[str]`` structurally, so nothing
in the annotation or a type checker objects. ``charter_code.run_judge`` coerced
it where this composition used to live and moving the composition out a layer
did not bring the handling along. Unreachable today -- the Charter path is
normalised before it arrives and no bundled plugin implements the verb -- but
this is the seam meant to carry generated judgements, and a bare string is a
plausible thing for one to answer. Coerced, with a test, mutation-checked.

Two records corrected. The type-only ``raven.contracts`` -> ``raven.agent``
edge, the first in the tree, is the price of moving ``Verdict`` and ``Intake``
off the paper; it costs nothing at import time and lint-imports does not see it
through ``exclude_type_checking_imports``, so it is now written beside the
contract it bends rather than left to be discovered. And the ledger entry for
the 3,200 -> 3,410 bump had been renamed but not refreshed, so it read as a
present-tense description of a file whose line count and exported names had
both moved; it is restored to the names it was written with, with a line saying
an entry is a dated record and not a description of today.

CONTEXT.md said a verb answers with "a list of names". None does --
``select_tools`` answers with definitions, which the paper spends a paragraph
insisting on.

Not taken, with reasons. The suggestion to read ``step.phase`` at
research_flow/flow.py:1000 instead of reconstructing from ``step.tools_ran``:
the two vocabularies coincide on ``after_iteration`` only.

    gate chain (_phase keys)   StepView.phase
    before_user_inbound        user_inbound
    before_iteration           iteration
    before_execute_tools       execute_tools
    after_iteration            after_iteration    <- the only member they share
    terminal_answerless        answerless
    after_send                 sent

That call site wants ``before_execute_tools``, the key ``_phase`` dispatches on,
where ``StepView.phase`` says ``execute_tools``, so substituting would route it
to a key with no gate behind it. The table is the reviewer's, built while
checking the reason rather than taking it. Two phase vocabularies overlapping on
exactly one member is a nasty shape: the shared member is the one a reader
samples, and it makes the two look like one vocabulary. The salvage narrowing belongs in
the PR description's Risk section and has been added there, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ypflll
ypflll force-pushed the refactor/participant_contract_closeout branch from 9120476 to 54679ad Compare September 18, 2026 09:35

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed the new 54679add revision after refreshing github/main to f633127e. This is a rebase-only revision: git range-diff marks all seven PR commits patch-identical to the previously reviewed series, and the three files changed by #493 no longer appear in the PR diff against the refreshed target. No review finding survives refutation because there is no new PR code delta.

Verification on this head:

  • git diff --check github/main...HEAD passed.
  • uv run pytest tests/test_participant_hook.py tests/test_agents_code_launcher.py tests/test_kernel_budget.py -x: 95 passed.
  • All 17 GitHub checks passed.
  • All seven existing review threads remain resolved; zero unresolved threads.

Coverage for this revision: repository rules and canonical context, the complete target diff via exact patch-series comparison, callers and compatibility already covered by the unchanged reviewed patches, test-strength preservation, architecture boundaries, and rebase history/integration with #493.

ypflll pushed a commit that referenced this pull request Sep 18, 2026
… becoming eighteen

Review on #495 (LivXue), at 3a1bf07. Five of its findings; two it raised are
answered rather than taken, below.

The judge disclosure had reached two of its four homes. The two it had not are
``ActionModule.ask_judge`` on the paper and ``DefaultAction.ask_judge``, and
those are the ones a reader of the role protocol meets first. They survived
because they phrase the claim without naming anything the earlier fix touched:
"composed from each participant's judge", "asked after whatever the caller
passes". There is one caller and it passes nothing. Both now say so.

The test meant to pin that could not fail for the reason it was written. It
collected what the participant was asked and then asserted only that the
Charter still refused -- true before this PR and true after any wiring anyone
might add -- and it built a ``ParticipantHook`` it then dropped, so the factory
never ran and the collection could not have been non-empty whatever the
production code did. It now asserts the argument the registry hands the role,
which is the fact the three docstrings rest on. Mutation-checked by wiring a
participant through ``_verifier_refusals``: the old assertion stays green, the
new one turns red.

``compose_judge`` exploded a one-sentence refusal into one refusal per
character: a bare ``str`` satisfies ``Sequence[str]`` structurally, so nothing
in the annotation or a type checker objects. ``charter_code.run_judge`` coerced
it where this composition used to live and moving the composition out a layer
did not bring the handling along. Unreachable today -- the Charter path is
normalised before it arrives and no bundled plugin implements the verb -- but
this is the seam meant to carry generated judgements, and a bare string is a
plausible thing for one to answer. Coerced, with a test, mutation-checked.

Two records corrected. The type-only ``raven.contracts`` -> ``raven.agent``
edge, the first in the tree, is the price of moving ``Verdict`` and ``Intake``
off the paper; it costs nothing at import time and lint-imports does not see it
through ``exclude_type_checking_imports``, so it is now written beside the
contract it bends rather than left to be discovered. And the ledger entry for
the 3,200 -> 3,410 bump had been renamed but not refreshed, so it read as a
present-tense description of a file whose line count and exported names had
both moved; it is restored to the names it was written with, with a line saying
an entry is a dated record and not a description of today.

CONTEXT.md said a verb answers with "a list of names". None does --
``select_tools`` answers with definitions, which the paper spends a paragraph
insisting on.

Not taken, with reasons. The suggestion to read ``step.phase`` at
research_flow/flow.py:1000 instead of reconstructing from ``step.tools_ran``:
the two vocabularies coincide on ``after_iteration`` only.

    gate chain (_phase keys)   StepView.phase
    before_user_inbound        user_inbound
    before_iteration           iteration
    before_execute_tools       execute_tools
    after_iteration            after_iteration    <- the only member they share
    terminal_answerless        answerless
    after_send                 sent

That call site wants ``before_execute_tools``, the key ``_phase`` dispatches on,
where ``StepView.phase`` says ``execute_tools``, so substituting would route it
to a key with no gate behind it. The table is the reviewer's, built while
checking the reason rather than taking it. Two phase vocabularies overlapping on
exactly one member is a nasty shape: the shared member is the one a reader
samples, and it makes the two look like one vocabulary. The salvage narrowing belongs in
the PR description's Risk section and has been added there, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ypflll
ypflll force-pushed the refactor/participant_contract_closeout branch from 54679ad to 7d6d6fb Compare September 18, 2026 10:31

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed the revision delta. 7d6d6fb6 is a rebase onto 45203a4a, whose only change since the prior base is .github/CODEOWNERS; it has no path overlap with this PR. git range-diff marks all seven PR commits patch-identical, so the previously reviewed code, callers, compatibility behavior, tests, and architecture boundaries are unchanged. No new finding has a concrete failure to report.

Current-head verification: git diff --check github/main...HEAD passed; uv run pytest tests/test_participant_hook.py tests/test_kernel_budget.py -x passed 25/25; all 17 GitHub checks passed; zero review threads are unresolved. Repository rules, canonical context, target diff, history, backward compatibility, test strength, and architecture were covered through the unchanged patch series plus the no-overlap base delta.

ypflll pushed a commit that referenced this pull request Sep 18, 2026
… becoming eighteen

Review on #495 (LivXue), at 3a1bf07. Five of its findings; two it raised are
answered rather than taken, below.

The judge disclosure had reached two of its four homes. The two it had not are
``ActionModule.ask_judge`` on the paper and ``DefaultAction.ask_judge``, and
those are the ones a reader of the role protocol meets first. They survived
because they phrase the claim without naming anything the earlier fix touched:
"composed from each participant's judge", "asked after whatever the caller
passes". There is one caller and it passes nothing. Both now say so.

The test meant to pin that could not fail for the reason it was written. It
collected what the participant was asked and then asserted only that the
Charter still refused -- true before this PR and true after any wiring anyone
might add -- and it built a ``ParticipantHook`` it then dropped, so the factory
never ran and the collection could not have been non-empty whatever the
production code did. It now asserts the argument the registry hands the role,
which is the fact the three docstrings rest on. Mutation-checked by wiring a
participant through ``_verifier_refusals``: the old assertion stays green, the
new one turns red.

``compose_judge`` exploded a one-sentence refusal into one refusal per
character: a bare ``str`` satisfies ``Sequence[str]`` structurally, so nothing
in the annotation or a type checker objects. ``charter_code.run_judge`` coerced
it where this composition used to live and moving the composition out a layer
did not bring the handling along. Unreachable today -- the Charter path is
normalised before it arrives and no bundled plugin implements the verb -- but
this is the seam meant to carry generated judgements, and a bare string is a
plausible thing for one to answer. Coerced, with a test, mutation-checked.

Two records corrected. The type-only ``raven.contracts`` -> ``raven.agent``
edge, the first in the tree, is the price of moving ``Verdict`` and ``Intake``
off the paper; it costs nothing at import time and lint-imports does not see it
through ``exclude_type_checking_imports``, so it is now written beside the
contract it bends rather than left to be discovered. And the ledger entry for
the 3,200 -> 3,410 bump had been renamed but not refreshed, so it read as a
present-tense description of a file whose line count and exported names had
both moved; it is restored to the names it was written with, with a line saying
an entry is a dated record and not a description of today.

CONTEXT.md said a verb answers with "a list of names". None does --
``select_tools`` answers with definitions, which the paper spends a paragraph
insisting on.

Not taken, with reasons. The suggestion to read ``step.phase`` at
research_flow/flow.py:1000 instead of reconstructing from ``step.tools_ran``:
the two vocabularies coincide on ``after_iteration`` only.

    gate chain (_phase keys)   StepView.phase
    before_user_inbound        user_inbound
    before_iteration           iteration
    before_execute_tools       execute_tools
    after_iteration            after_iteration    <- the only member they share
    terminal_answerless        answerless
    after_send                 sent

That call site wants ``before_execute_tools``, the key ``_phase`` dispatches on,
where ``StepView.phase`` says ``execute_tools``, so substituting would route it
to a key with no gate behind it. The table is the reviewer's, built while
checking the reason rather than taking it. Two phase vocabularies overlapping on
exactly one member is a nasty shape: the shared member is the one a reader
samples, and it makes the two look like one vocabulary. The salvage narrowing belongs in
the PR description's Risk section and has been added there, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ypflll
ypflll force-pushed the refactor/participant_contract_closeout branch from 7d6d6fb to 9a2a1df Compare September 18, 2026 15:13

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed the revision delta only. 9a2a1df9 rebases the same seven patch-identical commits onto c2c5f279; the new base changes only README.md and README.zh-CN.md, with no overlap with this PR. There is therefore no new code, caller, compatibility, test-strength, or architecture change to raise.

Current-head checks: git diff --check github/main...HEAD passed; the focused participant/kernel tests passed 25/25; zero review threads are unresolved. Twelve GitHub checks are green and the four unit shards are still running at review time.

Covered for this revision: repository rules and canonical context, the target diff and history through exact range-diff, base/PR overlap, callers and backward compatibility inherited unchanged, test preservation, and architecture constraints.

Comment thread raven/contracts/participant.py Resolved
@0xKT

0xKT commented Sep 19, 2026

Copy link
Copy Markdown
Member

Not a blocker -- separate from the review thread on raven/contracts/participant.py.

The kernel's first outward edge is forbidden by a contract the guard cannot see
(raven/contracts/harness.py:55)

Under if TYPE_CHECKING: the paper now reads
from raven.agent.harness.participants import Intake, Verdict, used in three return annotations
(ask_intake at :199 and :222, ask_review at :375).

pyproject's import contracts include one named the kernel stands alone, whose source_modules
list contains raven.contracts and whose forbidden_modules list contains raven.agent. So the
repo's own machine-readable law forbids exactly this edge. Measured:

as written, inside TYPE_CHECKING      -> lint-imports: Contracts: 10 kept, 0 broken
the same import moved out of the block -> lint-imports: the kernel stands alone BROKEN
                                          Contracts: 9 kept, 1 broken

So the law does catch this edge; the TYPE_CHECKING block is what it cannot see. At the merge base
raven/contracts has zero imports from raven.agent; at this head there is exactly one.

The budget docstring discloses it, in the same breath as the reason: "the first the kernel has,
taken knowingly when Verdict and Intake moved off the paper", and "lint-imports does not see
it, so it is written down here". That disclosure is the right instinct. What is missing is that the
next such edge will also pass, silently, because nothing changed in the guard.

Two ways to close it that I could not refute, both cheaper than they look:

  • annotate what actually crosses the boundary. The two-tier ledger's own comment says the answers
    are "plain mappings" and that "the host's own read of a verdict lives in the harness, not on the
    paper" -- if the paper annotates the mapping rather than the host's read model, the import is
    unnecessary and the edge disappears;
  • or keep the two shapes on the paper. They are plain dataclasses -- Verdict is six fields and
    one property, Intake three fields and no methods -- so this costs a few dozen lines on a
    surface whose ceiling this PR was already raising.

If neither is wanted, the third option is to make the guard see what was decided here: a check that
reads TYPE_CHECKING imports too, so the second edge is a decision rather than a precedent.

For the record, the parts of this change I checked hardest and found sound:

  • The export delta is net zero: raven/contracts goes 98 public names to 98, three out
    (AgentConduct, ConductFactory, Verdict) and three in (AgentParticipant, Answer,
    ParticipantFactory).
  • The rename is complete. Every surviving conduct in the tree is a deliberate one: the community
    CODE_OF_CONDUCT.md, raven-code's "Coding Conduct" prompts (which CONTEXT.md:195 explicitly
    calls "a different thing with a similar name"), pristine_conducts() which belongs to those
    prompts, and the dated historical entries in the budget docstring -- which this PR annotates as
    "a dated record of one bump, not a description of the file today".
  • The five ceiling entries each name what they bought and each record a measurement
    (3,444 / 3,515 / 3,528 / 3,542). I measured the surface independently: 3,427 lines at the merge
    base against a 3,440 ceiling, 3,542 at this head against 3,570.
  • Of the thirteen test files this PR touches, the suite is 363 passed with four failures, and all
    four are in the pre-existing failure set this checkout carries at the merge base.

ypflll pushed a commit that referenced this pull request Sep 19, 2026
… becoming eighteen

Review on #495 (LivXue), at 3a1bf07. Five of its findings; two it raised are
answered rather than taken, below.

The judge disclosure had reached two of its four homes. The two it had not are
``ActionModule.ask_judge`` on the paper and ``DefaultAction.ask_judge``, and
those are the ones a reader of the role protocol meets first. They survived
because they phrase the claim without naming anything the earlier fix touched:
"composed from each participant's judge", "asked after whatever the caller
passes". There is one caller and it passes nothing. Both now say so.

The test meant to pin that could not fail for the reason it was written. It
collected what the participant was asked and then asserted only that the
Charter still refused -- true before this PR and true after any wiring anyone
might add -- and it built a ``ParticipantHook`` it then dropped, so the factory
never ran and the collection could not have been non-empty whatever the
production code did. It now asserts the argument the registry hands the role,
which is the fact the three docstrings rest on. Mutation-checked by wiring a
participant through ``_verifier_refusals``: the old assertion stays green, the
new one turns red.

``compose_judge`` exploded a one-sentence refusal into one refusal per
character: a bare ``str`` satisfies ``Sequence[str]`` structurally, so nothing
in the annotation or a type checker objects. ``charter_code.run_judge`` coerced
it where this composition used to live and moving the composition out a layer
did not bring the handling along. Unreachable today -- the Charter path is
normalised before it arrives and no bundled plugin implements the verb -- but
this is the seam meant to carry generated judgements, and a bare string is a
plausible thing for one to answer. Coerced, with a test, mutation-checked.

Two records corrected. The type-only ``raven.contracts`` -> ``raven.agent``
edge, the first in the tree, is the price of moving ``Verdict`` and ``Intake``
off the paper; it costs nothing at import time and lint-imports does not see it
through ``exclude_type_checking_imports``, so it is now written beside the
contract it bends rather than left to be discovered. And the ledger entry for
the 3,200 -> 3,410 bump had been renamed but not refreshed, so it read as a
present-tense description of a file whose line count and exported names had
both moved; it is restored to the names it was written with, with a line saying
an entry is a dated record and not a description of today.

CONTEXT.md said a verb answers with "a list of names". None does --
``select_tools`` answers with definitions, which the paper spends a paragraph
insisting on.

Not taken, with reasons. The suggestion to read ``step.phase`` at
research_flow/flow.py:1000 instead of reconstructing from ``step.tools_ran``:
the two vocabularies coincide on ``after_iteration`` only.

    gate chain (_phase keys)   StepView.phase
    before_user_inbound        user_inbound
    before_iteration           iteration
    before_execute_tools       execute_tools
    after_iteration            after_iteration    <- the only member they share
    terminal_answerless        answerless
    after_send                 sent

That call site wants ``before_execute_tools``, the key ``_phase`` dispatches on,
where ``StepView.phase`` says ``execute_tools``, so substituting would route it
to a key with no gate behind it. The table is the reviewer's, built while
checking the reason rather than taking it. Two phase vocabularies overlapping on
exactly one member is a nasty shape: the shared member is the one a reader
samples, and it makes the two look like one vocabulary. The salvage narrowing belongs in
the PR description's Risk section and has been added there, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ypflll
ypflll force-pushed the refactor/participant_contract_closeout branch from 9a2a1df to 15a39d6 Compare September 19, 2026 07:07

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed the revision delta. 15a39d63 rebases the same seven patch-identical commits onto the docs-site merge at 3dd06f6e. The only base/PR path overlap is pyproject.toml: the base adds docs dependencies and configuration, while this PR retains its previously reviewed import-contract disclosure. The combined configuration is intact; I found no new code, caller, compatibility, test-strength, or architecture failure introduced by the rebase.

Current-head verification: git diff --check github/main...HEAD passed; uv run pytest tests/test_participant_hook.py tests/test_kernel_budget.py -x passed 25/25; uv run lint-imports kept all 10 contracts with 0 broken. Four unit shards remain pending in GitHub CI; the other reported checks pass.

The one unresolved thread is @0xKT’s blocker about publishing judge before production wiring. It is not my thread or judgement to re-grade, and I have left it open. Covered here: the replacement AGENTS.md rules and canonical context, target diff and history via exact range-diff, the pyproject.toml overlap, callers, backward compatibility, test preservation, and architecture contracts.

ypflll pushed a commit that referenced this pull request Sep 19, 2026
… becoming eighteen

Review on #495 (LivXue), at 3a1bf07. Five of its findings; two it raised are
answered rather than taken, below.

The judge disclosure had reached two of its four homes. The two it had not are
``ActionModule.ask_judge`` on the paper and ``DefaultAction.ask_judge``, and
those are the ones a reader of the role protocol meets first. They survived
because they phrase the claim without naming anything the earlier fix touched:
"composed from each participant's judge", "asked after whatever the caller
passes". There is one caller and it passes nothing. Both now say so.

The test meant to pin that could not fail for the reason it was written. It
collected what the participant was asked and then asserted only that the
Charter still refused -- true before this PR and true after any wiring anyone
might add -- and it built a ``ParticipantHook`` it then dropped, so the factory
never ran and the collection could not have been non-empty whatever the
production code did. It now asserts the argument the registry hands the role,
which is the fact the three docstrings rest on. Mutation-checked by wiring a
participant through ``_verifier_refusals``: the old assertion stays green, the
new one turns red.

``compose_judge`` exploded a one-sentence refusal into one refusal per
character: a bare ``str`` satisfies ``Sequence[str]`` structurally, so nothing
in the annotation or a type checker objects. ``charter_code.run_judge`` coerced
it where this composition used to live and moving the composition out a layer
did not bring the handling along. Unreachable today -- the Charter path is
normalised before it arrives and no bundled plugin implements the verb -- but
this is the seam meant to carry generated judgements, and a bare string is a
plausible thing for one to answer. Coerced, with a test, mutation-checked.

Two records corrected. The type-only ``raven.contracts`` -> ``raven.agent``
edge, the first in the tree, is the price of moving ``Verdict`` and ``Intake``
off the paper; it costs nothing at import time and lint-imports does not see it
through ``exclude_type_checking_imports``, so it is now written beside the
contract it bends rather than left to be discovered. And the ledger entry for
the 3,200 -> 3,410 bump had been renamed but not refreshed, so it read as a
present-tense description of a file whose line count and exported names had
both moved; it is restored to the names it was written with, with a line saying
an entry is a dated record and not a description of today.

CONTEXT.md said a verb answers with "a list of names". None does --
``select_tools`` answers with definitions, which the paper spends a paragraph
insisting on.

Not taken, with reasons. The suggestion to read ``step.phase`` at
research_flow/flow.py:1000 instead of reconstructing from ``step.tools_ran``:
the two vocabularies coincide on ``after_iteration`` only.

    gate chain (_phase keys)   StepView.phase
    before_user_inbound        user_inbound
    before_iteration           iteration
    before_execute_tools       execute_tools
    after_iteration            after_iteration    <- the only member they share
    terminal_answerless        answerless
    after_send                 sent

That call site wants ``before_execute_tools``, the key ``_phase`` dispatches on,
where ``StepView.phase`` says ``execute_tools``, so substituting would route it
to a key with no gate behind it. The table is the reviewer's, built while
checking the reason rather than taking it. Two phase vocabularies overlapping on
exactly one member is a nasty shape: the shared member is the one a reader
samples, and it makes the two look like one vocabulary. The salvage narrowing belongs in
the PR description's Risk section and has been added there, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ypflll
ypflll force-pushed the refactor/participant_contract_closeout branch from b30a3d4 to 8e46ac8 Compare September 19, 2026 14:05

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed the new revision and the whole record. 8e46ac842 is a no-overlap rebase onto 0eb8b41be: all seven change commits are patch-identical, while the new base commit is the unrelated configuration fix from #512. @0xKT has independently re-verified the production judge chain, withdrawn the ground of the prior blocker, and resolved the thread; GitHub now reports zero unresolved review threads.

Verification: git range-diff reports seven = pairs; git diff --check github/main...HEAD passed; uv run pytest tests/test_participant_hook.py tests/test_kernel_budget.py -x passed (25 tests). Every completed GitHub check passes, with four unit shards still pending. I covered the repository rules and context, diff and history, callers, backward compatibility, test strength, and architecture boundaries; this revision adds no new concern.

ypflll pushed a commit that referenced this pull request Sep 19, 2026
… becoming eighteen

Review on #495 (LivXue), at 3a1bf07. Five of its findings; two it raised are
answered rather than taken, below.

The judge disclosure had reached two of its four homes. The two it had not are
``ActionModule.ask_judge`` on the paper and ``DefaultAction.ask_judge``, and
those are the ones a reader of the role protocol meets first. They survived
because they phrase the claim without naming anything the earlier fix touched:
"composed from each participant's judge", "asked after whatever the caller
passes". There is one caller and it passes nothing. Both now say so.

The test meant to pin that could not fail for the reason it was written. It
collected what the participant was asked and then asserted only that the
Charter still refused -- true before this PR and true after any wiring anyone
might add -- and it built a ``ParticipantHook`` it then dropped, so the factory
never ran and the collection could not have been non-empty whatever the
production code did. It now asserts the argument the registry hands the role,
which is the fact the three docstrings rest on. Mutation-checked by wiring a
participant through ``_verifier_refusals``: the old assertion stays green, the
new one turns red.

``compose_judge`` exploded a one-sentence refusal into one refusal per
character: a bare ``str`` satisfies ``Sequence[str]`` structurally, so nothing
in the annotation or a type checker objects. ``charter_code.run_judge`` coerced
it where this composition used to live and moving the composition out a layer
did not bring the handling along. Unreachable today -- the Charter path is
normalised before it arrives and no bundled plugin implements the verb -- but
this is the seam meant to carry generated judgements, and a bare string is a
plausible thing for one to answer. Coerced, with a test, mutation-checked.

Two records corrected. The type-only ``raven.contracts`` -> ``raven.agent``
edge, the first in the tree, is the price of moving ``Verdict`` and ``Intake``
off the paper; it costs nothing at import time and lint-imports does not see it
through ``exclude_type_checking_imports``, so it is now written beside the
contract it bends rather than left to be discovered. And the ledger entry for
the 3,200 -> 3,410 bump had been renamed but not refreshed, so it read as a
present-tense description of a file whose line count and exported names had
both moved; it is restored to the names it was written with, with a line saying
an entry is a dated record and not a description of today.

CONTEXT.md said a verb answers with "a list of names". None does --
``select_tools`` answers with definitions, which the paper spends a paragraph
insisting on.

Not taken, with reasons. The suggestion to read ``step.phase`` at
research_flow/flow.py:1000 instead of reconstructing from ``step.tools_ran``:
the two vocabularies coincide on ``after_iteration`` only.

    gate chain (_phase keys)   StepView.phase
    before_user_inbound        user_inbound
    before_iteration           iteration
    before_execute_tools       execute_tools
    after_iteration            after_iteration    <- the only member they share
    terminal_answerless        answerless
    after_send                 sent

That call site wants ``before_execute_tools``, the key ``_phase`` dispatches on,
where ``StepView.phase`` says ``execute_tools``, so substituting would route it
to a key with no gate behind it. The table is the reviewer's, built while
checking the reason rather than taking it. Two phase vocabularies overlapping on
exactly one member is a nasty shape: the shared member is the one a reader
samples, and it makes the two look like one vocabulary. The salvage narrowing belongs in
the PR description's Risk section and has been added there, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ypflll
ypflll force-pushed the refactor/participant_contract_closeout branch from 8e46ac8 to 1e8fae4 Compare September 19, 2026 14:44

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed the revision delta. 1e8fae4b4 rebases the same seven patch-identical commits onto b4b0604f2; the sole new base commit is the unrelated #517 documentation showcase, and it shares no changed paths with this PR. There is no new code, caller, compatibility, test-strength, or architecture concern.

Verification: git range-diff reports seven = pairs; git diff --check github/main...HEAD passed; uv run pytest tests/test_participant_hook.py tests/test_kernel_budget.py -x passed (25 tests). All completed GitHub checks pass, with TUI, UI RPC, and four unit jobs pending. GitHub reports zero unresolved review threads. I covered the repository rules and context, diff and history, callers, backward compatibility, test assertions, and architecture boundaries.

ypflll pushed a commit that referenced this pull request Sep 19, 2026
… becoming eighteen

Review on #495 (LivXue), at 3a1bf07. Five of its findings; two it raised are
answered rather than taken, below.

The judge disclosure had reached two of its four homes. The two it had not are
``ActionModule.ask_judge`` on the paper and ``DefaultAction.ask_judge``, and
those are the ones a reader of the role protocol meets first. They survived
because they phrase the claim without naming anything the earlier fix touched:
"composed from each participant's judge", "asked after whatever the caller
passes". There is one caller and it passes nothing. Both now say so.

The test meant to pin that could not fail for the reason it was written. It
collected what the participant was asked and then asserted only that the
Charter still refused -- true before this PR and true after any wiring anyone
might add -- and it built a ``ParticipantHook`` it then dropped, so the factory
never ran and the collection could not have been non-empty whatever the
production code did. It now asserts the argument the registry hands the role,
which is the fact the three docstrings rest on. Mutation-checked by wiring a
participant through ``_verifier_refusals``: the old assertion stays green, the
new one turns red.

``compose_judge`` exploded a one-sentence refusal into one refusal per
character: a bare ``str`` satisfies ``Sequence[str]`` structurally, so nothing
in the annotation or a type checker objects. ``charter_code.run_judge`` coerced
it where this composition used to live and moving the composition out a layer
did not bring the handling along. Unreachable today -- the Charter path is
normalised before it arrives and no bundled plugin implements the verb -- but
this is the seam meant to carry generated judgements, and a bare string is a
plausible thing for one to answer. Coerced, with a test, mutation-checked.

Two records corrected. The type-only ``raven.contracts`` -> ``raven.agent``
edge, the first in the tree, is the price of moving ``Verdict`` and ``Intake``
off the paper; it costs nothing at import time and lint-imports does not see it
through ``exclude_type_checking_imports``, so it is now written beside the
contract it bends rather than left to be discovered. And the ledger entry for
the 3,200 -> 3,410 bump had been renamed but not refreshed, so it read as a
present-tense description of a file whose line count and exported names had
both moved; it is restored to the names it was written with, with a line saying
an entry is a dated record and not a description of today.

CONTEXT.md said a verb answers with "a list of names". None does --
``select_tools`` answers with definitions, which the paper spends a paragraph
insisting on.

Not taken, with reasons. The suggestion to read ``step.phase`` at
research_flow/flow.py:1000 instead of reconstructing from ``step.tools_ran``:
the two vocabularies coincide on ``after_iteration`` only.

    gate chain (_phase keys)   StepView.phase
    before_user_inbound        user_inbound
    before_iteration           iteration
    before_execute_tools       execute_tools
    after_iteration            after_iteration    <- the only member they share
    terminal_answerless        answerless
    after_send                 sent

That call site wants ``before_execute_tools``, the key ``_phase`` dispatches on,
where ``StepView.phase`` says ``execute_tools``, so substituting would route it
to a key with no gate behind it. The table is the reviewer's, built while
checking the reason rather than taking it. Two phase vocabularies overlapping on
exactly one member is a nasty shape: the shared member is the one a reader
samples, and it makes the two look like one vocabulary. The salvage narrowing belongs in
the PR description's Risk section and has been added there, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ypflll
ypflll force-pushed the refactor/participant_contract_closeout branch from 1e8fae4 to 9bfbbad Compare September 19, 2026 15:41

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed the revision delta. 9bfbbadbd rebases the same seven patch-identical commits onto 7ab685f60. The new base has one shared path, pyproject.toml, but the changes are independent: main adds jieba to the docs dependency group, while this PR adds the import-boundary rationale later in the file. Both survive intact and there is no semantic interaction.

Verification: git range-diff reports seven = pairs; git diff --check github/main...HEAD passed; uv run pytest tests/test_participant_hook.py tests/test_kernel_budget.py -x passed (25 tests). All completed GitHub checks pass, with four unit jobs pending, and GitHub reports zero unresolved review threads. I covered the updated repository rules and context, diff and history, the overlapping file, callers, backward compatibility, test assertions, and architecture boundaries.

ypflll pushed a commit that referenced this pull request Sep 19, 2026
… becoming eighteen

Review on #495 (LivXue), at 3a1bf07. Five of its findings; two it raised are
answered rather than taken, below.

The judge disclosure had reached two of its four homes. The two it had not are
``ActionModule.ask_judge`` on the paper and ``DefaultAction.ask_judge``, and
those are the ones a reader of the role protocol meets first. They survived
because they phrase the claim without naming anything the earlier fix touched:
"composed from each participant's judge", "asked after whatever the caller
passes". There is one caller and it passes nothing. Both now say so.

The test meant to pin that could not fail for the reason it was written. It
collected what the participant was asked and then asserted only that the
Charter still refused -- true before this PR and true after any wiring anyone
might add -- and it built a ``ParticipantHook`` it then dropped, so the factory
never ran and the collection could not have been non-empty whatever the
production code did. It now asserts the argument the registry hands the role,
which is the fact the three docstrings rest on. Mutation-checked by wiring a
participant through ``_verifier_refusals``: the old assertion stays green, the
new one turns red.

``compose_judge`` exploded a one-sentence refusal into one refusal per
character: a bare ``str`` satisfies ``Sequence[str]`` structurally, so nothing
in the annotation or a type checker objects. ``charter_code.run_judge`` coerced
it where this composition used to live and moving the composition out a layer
did not bring the handling along. Unreachable today -- the Charter path is
normalised before it arrives and no bundled plugin implements the verb -- but
this is the seam meant to carry generated judgements, and a bare string is a
plausible thing for one to answer. Coerced, with a test, mutation-checked.

Two records corrected. The type-only ``raven.contracts`` -> ``raven.agent``
edge, the first in the tree, is the price of moving ``Verdict`` and ``Intake``
off the paper; it costs nothing at import time and lint-imports does not see it
through ``exclude_type_checking_imports``, so it is now written beside the
contract it bends rather than left to be discovered. And the ledger entry for
the 3,200 -> 3,410 bump had been renamed but not refreshed, so it read as a
present-tense description of a file whose line count and exported names had
both moved; it is restored to the names it was written with, with a line saying
an entry is a dated record and not a description of today.

CONTEXT.md said a verb answers with "a list of names". None does --
``select_tools`` answers with definitions, which the paper spends a paragraph
insisting on.

Not taken, with reasons. The suggestion to read ``step.phase`` at
research_flow/flow.py:1000 instead of reconstructing from ``step.tools_ran``:
the two vocabularies coincide on ``after_iteration`` only.

    gate chain (_phase keys)   StepView.phase
    before_user_inbound        user_inbound
    before_iteration           iteration
    before_execute_tools       execute_tools
    after_iteration            after_iteration    <- the only member they share
    terminal_answerless        answerless
    after_send                 sent

That call site wants ``before_execute_tools``, the key ``_phase`` dispatches on,
where ``StepView.phase`` says ``execute_tools``, so substituting would route it
to a key with no gate behind it. The table is the reviewer's, built while
checking the reason rather than taking it. Two phase vocabularies overlapping on
exactly one member is a nasty shape: the shared member is the one a reader
samples, and it makes the two look like one vocabulary. The salvage narrowing belongs in
the PR description's Risk section and has been added there, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ypflll
ypflll force-pushed the refactor/participant_contract_closeout branch from 9bfbbad to 85b2aea Compare September 19, 2026 16:08

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed the revision delta. 85b2aea2f rebases the same seven patch-identical commits onto 900fa38a3; the sole new base commit updates an unrelated documentation showcase chart and shares no changed paths with this PR. No code, caller, compatibility, test-strength, or architecture concern is introduced.

Verification: git range-diff reports seven = pairs; git diff --check github/main...HEAD passed; uv run pytest tests/test_participant_hook.py tests/test_kernel_budget.py -x passed (25 tests). Every completed GitHub check passes, with four unit jobs pending, and GitHub reports zero unresolved review threads. I covered the repository rules and context, diff and history, callers, backward compatibility, test assertions, and architecture boundaries.

ypflll pushed a commit that referenced this pull request Sep 20, 2026
… becoming eighteen

Review on #495 (LivXue), at 3a1bf07. Five of its findings; two it raised are
answered rather than taken, below.

The judge disclosure had reached two of its four homes. The two it had not are
``ActionModule.ask_judge`` on the paper and ``DefaultAction.ask_judge``, and
those are the ones a reader of the role protocol meets first. They survived
because they phrase the claim without naming anything the earlier fix touched:
"composed from each participant's judge", "asked after whatever the caller
passes". There is one caller and it passes nothing. Both now say so.

The test meant to pin that could not fail for the reason it was written. It
collected what the participant was asked and then asserted only that the
Charter still refused -- true before this PR and true after any wiring anyone
might add -- and it built a ``ParticipantHook`` it then dropped, so the factory
never ran and the collection could not have been non-empty whatever the
production code did. It now asserts the argument the registry hands the role,
which is the fact the three docstrings rest on. Mutation-checked by wiring a
participant through ``_verifier_refusals``: the old assertion stays green, the
new one turns red.

``compose_judge`` exploded a one-sentence refusal into one refusal per
character: a bare ``str`` satisfies ``Sequence[str]`` structurally, so nothing
in the annotation or a type checker objects. ``charter_code.run_judge`` coerced
it where this composition used to live and moving the composition out a layer
did not bring the handling along. Unreachable today -- the Charter path is
normalised before it arrives and no bundled plugin implements the verb -- but
this is the seam meant to carry generated judgements, and a bare string is a
plausible thing for one to answer. Coerced, with a test, mutation-checked.

Two records corrected. The type-only ``raven.contracts`` -> ``raven.agent``
edge, the first in the tree, is the price of moving ``Verdict`` and ``Intake``
off the paper; it costs nothing at import time and lint-imports does not see it
through ``exclude_type_checking_imports``, so it is now written beside the
contract it bends rather than left to be discovered. And the ledger entry for
the 3,200 -> 3,410 bump had been renamed but not refreshed, so it read as a
present-tense description of a file whose line count and exported names had
both moved; it is restored to the names it was written with, with a line saying
an entry is a dated record and not a description of today.

CONTEXT.md said a verb answers with "a list of names". None does --
``select_tools`` answers with definitions, which the paper spends a paragraph
insisting on.

Not taken, with reasons. The suggestion to read ``step.phase`` at
research_flow/flow.py:1000 instead of reconstructing from ``step.tools_ran``:
the two vocabularies coincide on ``after_iteration`` only.

    gate chain (_phase keys)   StepView.phase
    before_user_inbound        user_inbound
    before_iteration           iteration
    before_execute_tools       execute_tools
    after_iteration            after_iteration    <- the only member they share
    terminal_answerless        answerless
    after_send                 sent

That call site wants ``before_execute_tools``, the key ``_phase`` dispatches on,
where ``StepView.phase`` says ``execute_tools``, so substituting would route it
to a key with no gate behind it. The table is the reviewer's, built while
checking the reason rather than taking it. Two phase vocabularies overlapping on
exactly one member is a nasty shape: the shared member is the one a reader
samples, and it makes the two look like one vocabulary. The salvage narrowing belongs in
the PR description's Risk section and has been added there, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ypflll
ypflll force-pushed the refactor/participant_contract_closeout branch from 85b2aea to 58a6bc0 Compare September 20, 2026 02:11

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed the revision delta. 58a6bc0fc rebases the same seven patch-identical commits onto 04eb8a31d; the two new base commits are unrelated documentation presentation changes and share no changed paths with this PR. No code, caller, compatibility, test-strength, or architecture concern is introduced.

Verification: git range-diff reports seven = pairs; git diff --check github/main...HEAD passed; uv run pytest tests/test_participant_hook.py tests/test_kernel_budget.py -x passed (25 tests). Every completed GitHub check passes, with four unit jobs pending, and GitHub reports zero unresolved review threads. I covered the repository rules and context, diff and history, callers, backward compatibility, test assertions, and architecture boundaries.

Comment thread raven/agent/harness/participants.py Resolved
yao pengfei and others added 8 commits September 20, 2026 02:55
…serve

Four closeouts on the conduct contract, each narrowing what the paper claims.

``observe`` is gone. A verb with no return value decides nothing, so its only
possible effect is the state the invariants forbid a participant from keeping.
Nothing in the tree implemented it; the seat asked it once per iteration and
threw the answer away.

``outbound`` says on its own docstring that it is the one verb with no module
seat, and why: the four roles name the turn's decisions -- the window, the
guidance, the tools, the model call -- and none of them owns what a turn
finally delivers to a person. That also makes it the one verb a generated
participant cannot be handed, since there would be nothing to compose it with
and nothing to vet it.

``StepView`` gains ``phase``. Two verbs are asked at more than one moment --
``review`` before a step's tools run and again after, ``advise`` before the
call and after it -- and a participant that behaves differently at each was
left to infer which time it was from whichever other field happened to be set.
``tools_ran`` is now a property derived from ``phase``, so the two cannot
disagree.

The four delegating role methods are named apart from the verbs they ask:
``read_inbound`` asks ``intake``, ``guide`` asks ``advise``, ``judge_step``
asks ``review``, ``rescue`` asks ``salvage``. One name per layer, so a reader
of either knows which side of the seam they are on without checking.

Contracts ceiling 3,420 -> 3,460 with its review paragraph.

Verified: full suite 23037 passed, no failure the base branch does not also
show; ruff, format and import contracts clean; the four scripted A/Bs against
the parent are byte-identical.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
(cherry picked from commit 152a177e2c9ae10136e772b6023f4a08d41843e9)
``system_addendum``, ``archive`` and ``select_tools`` were rendered straight
onto the hook decision. Each was therefore the one judgement in the set with
nowhere to compose a second participant, nowhere to vet what came back, and
nothing a replacement role could decide -- which is also what would have made
them unable to carry a generated participant later.

They join the roles the other four already sit on:

- ``MemoryModule.compose_addendum`` composes what a call adds to the system
  message, because the system message is part of the window Memory assembles.
  The splicing stays with the seat: a role says what the text is, the shell
  says where it goes and takes it back out next call.
- ``MemoryModule.file_record`` merges what the turn's record is stamped with,
  by observer name, so two participants stamping the same name both survive.
- ``CapabilityModule.offer`` threads the tool array through each participant.

The tool seat is deliberately not the point where narrowing takes effect.
``select`` still is, and ``ToolRegistry.execute`` still adjudicates every call,
so a participant narrows after the product has spoken and never instead of it.

A test now drives all six phases against a harness of stubs and asserts every
one of the seven seated verbs went through its role. Contracts ceiling 3,460 ->
3,500 with its review paragraph.

Verified: full suite 23037 passed, no failure the base branch does not also
show; ruff, format and import contracts clean; the four scripted A/Bs against
the parent are byte-identical.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
(cherry picked from commit cf3641443fa52838297ce04a868898bf300cb62f)
…reads it

``review``, ``intake``, ``system_addendum`` and ``salvage`` handed back host
classes, which only a plugin can build. A participant is not only a plugin: a
dispatch's own judgements and anything generated for one answer the same verbs,
and a generated function can build a mapping where it cannot build a frozen
dataclass from the host's own package. So the verbs answer with mappings, and
the host reads them.

``Accept``, ``Resample``, ``End`` and ``Intake`` keep their names and write
those mappings, so a hand-written conduct still says ``Resample("too thin")``
rather than spelling out keys. ``Answer`` names the shape. ``salvage`` answers
with the reply text rather than with whatever object the host happened to
accept, so what a generated participant may say is what every participant may
say.

The reading is one place -- ``read_verdict`` and ``read_intake`` in the harness,
beside the merge rules they feed -- and it vets rather than trusts. An answer
this host cannot read is silence: a verdict it cannot parse is an accept, an
intake it cannot parse leaves the turn's own words alone, and a salvage that is
not text is not a reply. That is the third invariant where it matters most,
since two of these verbs can halt a turn and a participant must not be able to
halt one by answering badly.

The host's own ``Verdict`` and ``Intake`` move to the harness, where the shell
reads them, and off the paper the participants implement. The seat reads
whatever a replaced role hands over, so a role may answer in either shape.

``select_tools`` stays a definition array, deliberately out of step with the
Charter's name tuple, and the paper now says why: a name is enough to narrow
and not enough to contribute, and the tool the research flow adds for one
iteration has no registry entry to look a schema up from.

Two tests carry the invariant: a participant answering nonsense is silence at
every verb that could halt a turn, and a participant writing the keys by hand
is answering the same thing as one using the builders.

Verified: full suite 23038 passed, no failure the base branch does not also
show; ruff, format and import contracts clean; the four scripted A/Bs against
the parent are byte-identical.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
(cherry picked from commit 4b9fdf5459aa64dcee262c8026a46594efcdf6fd)
…ing a role reads

``judge`` was the one external judgement that never went through the
participant list: the Action role reached into the dispatch scope and read the
Charter's ``checks`` and ``code`` itself. That left the tree with two ways an
outside judgement reaches a call -- the list for everything else, a private
read for this one -- and no seat a generated judgement could join.

``AgentConduct.judge`` is now a verb like the others. It is the one synchronous
verb, because it runs before every tool dispatch and ahead of the permission
gate, where anything that blocked on the network would cost the turn rather
than the call; it was already the one verb answering in pure data, a list of
sentences.

The Charter answers it through a thin shell (``charter.participants()``), and
``ActionModule.judge`` takes the list. The merge is a veto: the first
participant that refuses decides and the rest are not asked, since one refusal
is enough and two reasons for one refusal read as confusion. Order is product
first, dispatch after, so where both would refuse it is the product's own
wording the model reads.

Nothing about what is refused changes: a bare registry still enforces, the
declarative rules and the compiled judge are the same code behind the shell,
and the four scripted A/Bs against the parent are byte-identical.

Contracts ceiling 3,500 -> 3,530 with its review paragraph.

Verified: full suite green with no failure the base branch does not also show;
ruff, format and import contracts clean; three tests cover the new shape -- a
dispatch with no judgements brings no participant, a product's rules speak
first, and a silent participant does not stop the next one being asked.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
(cherry picked from commit 239f0b9e279fb25889cee6b74d182e0b57e1d208)
…pant

Two renames that were one confusion each.

**The four asking methods take an ask_ prefix.** ``ActionModule.review`` and
``DesignParticipant.review`` were the same word for the two halves of one
exchange: one asks everybody and merges, the other answers for itself. The
asking half is now ``ask_intake`` / ``ask_advice`` / ``ask_review`` /
``ask_salvage``, which pairs each visibly with the verb it asks and says in the
name that the method does not judge anything itself.

**``Conduct`` becomes ``Participant``.** Two reasons, and the second is the one
that forced it. The word was already taken: raven-code's launcher has had
``render_conduct`` / ``conduct_source`` / ``seed_conduct`` / ``CONDUCTS`` for
the text a coding agent reads, so the tree carried one word for two unrelated
things. And ``participant`` is what the architecture already calls this side --
the call chain is shell, modules, participants -- which is the word a Charter's
own shell was already written under (``CharterParticipant``).

``AgentParticipant``, ``ParticipantHook``, ``ParticipantFactory``, the five
plugin classes, the ``participants`` parameter, the four files, and the
per-turn seat key. The launcher's own ``conduct`` words are untouched: they
mean the other thing.

Done symbol by symbol rather than by replacement, which is what caught the one
real defect: with the parameter named ``participants``, ``charter.participants()``
was shadowed by it and ``[*participants, *participants()]`` called the tuple.
Ruff was clean on that line; only the suite found it. The charter's own function
is ``charter_participants`` now.

Verified: full suite 23044 passed; the five failures are this machine's
(no browser, no node, plugin extras unbuilt) and stand on the base branch too;
ruff, format and import contracts clean.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
(cherry picked from commit 87c86c7ca78323164408636e406be1090f063693)
Read back over the seam rather than over the diff, and two claims turned out
wider than the tree.

``judge`` read as though a plugin's override were asked before a dispatch's own
rules. It is not, and cannot be as written: the party that asks the verb is
``ToolRegistry.execute``, which is not the hook chain and holds no handle on
this turn's participants, so the list it composes holds only the Charter. The
merge order is real and the test for it is real; what was missing is that
nothing puts a plugin in the list. Wiring one through is a turn-scoped register
this change does not add, and it would start refusing calls that run today, so
it is named on the paper instead of slipped in. A test pins the current
behaviour, so whoever wires it has to come back and change the prose.

``max_iterations`` lost its docstring to the ``tools_ran`` property, which was
inserted between the field and the string documenting it.

Two tests for claims nothing was checking. ``phase`` and the ``tools_ran``
derived from it had no assertion anywhere, and three of the five bundled
plugins branch on ``tools_ran`` -- ppt on its close, oncall on its counter,
research on which gate runs -- so a mistyped phase string at one of the six
call sites would have changed what they do with a green suite. The read-only
test now also covers ``mode_overlay``, the one frozen field it skipped. All
three assertions were mutation-checked: breaking each turns exactly its own
test red.

No executable line changes: the contract module's AST, with docstrings
stripped, is identical to its parent's.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… becoming eighteen

Review on #495 (LivXue), at 3a1bf07. Five of its findings; two it raised are
answered rather than taken, below.

The judge disclosure had reached two of its four homes. The two it had not are
``ActionModule.ask_judge`` on the paper and ``DefaultAction.ask_judge``, and
those are the ones a reader of the role protocol meets first. They survived
because they phrase the claim without naming anything the earlier fix touched:
"composed from each participant's judge", "asked after whatever the caller
passes". There is one caller and it passes nothing. Both now say so.

The test meant to pin that could not fail for the reason it was written. It
collected what the participant was asked and then asserted only that the
Charter still refused -- true before this PR and true after any wiring anyone
might add -- and it built a ``ParticipantHook`` it then dropped, so the factory
never ran and the collection could not have been non-empty whatever the
production code did. It now asserts the argument the registry hands the role,
which is the fact the three docstrings rest on. Mutation-checked by wiring a
participant through ``_verifier_refusals``: the old assertion stays green, the
new one turns red.

``compose_judge`` exploded a one-sentence refusal into one refusal per
character: a bare ``str`` satisfies ``Sequence[str]`` structurally, so nothing
in the annotation or a type checker objects. ``charter_code.run_judge`` coerced
it where this composition used to live and moving the composition out a layer
did not bring the handling along. Unreachable today -- the Charter path is
normalised before it arrives and no bundled plugin implements the verb -- but
this is the seam meant to carry generated judgements, and a bare string is a
plausible thing for one to answer. Coerced, with a test, mutation-checked.

Two records corrected. The type-only ``raven.contracts`` -> ``raven.agent``
edge, the first in the tree, is the price of moving ``Verdict`` and ``Intake``
off the paper; it costs nothing at import time and lint-imports does not see it
through ``exclude_type_checking_imports``, so it is now written beside the
contract it bends rather than left to be discovered. And the ledger entry for
the 3,200 -> 3,410 bump had been renamed but not refreshed, so it read as a
present-tense description of a file whose line count and exported names had
both moved; it is restored to the names it was written with, with a line saying
an entry is a dated record and not a description of today.

CONTEXT.md said a verb answers with "a list of names". None does --
``select_tools`` answers with definitions, which the paper spends a paragraph
insisting on.

Not taken, with reasons. The suggestion to read ``step.phase`` at
research_flow/flow.py:1000 instead of reconstructing from ``step.tools_ran``:
the two vocabularies coincide on ``after_iteration`` only.

    gate chain (_phase keys)   StepView.phase
    before_user_inbound        user_inbound
    before_iteration           iteration
    before_execute_tools       execute_tools
    after_iteration            after_iteration    <- the only member they share
    terminal_answerless        answerless
    after_send                 sent

That call site wants ``before_execute_tools``, the key ``_phase`` dispatches on,
where ``StepView.phase`` says ``execute_tools``, so substituting would route it
to a key with no gate behind it. The table is the reviewer's, built while
checking the reason rather than taking it. Two phase vocabularies overlapping on
exactly one member is a nasty shape: the shared member is the one a reader
samples, and it makes the two look like one vocabulary. The salvage narrowing belongs in
the PR description's Risk section and has been added there, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ypflll
ypflll force-pushed the refactor/participant_contract_closeout branch from 58a6bc0 to 448f6f3 Compare September 20, 2026 02:56
@ypflll
ypflll requested a review from 0xKT September 20, 2026 02:59

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed the substantive eighth commit, fix(agent): preserve scalar observer values, as well as the rebased series. compose_record now retains non-mapping observer values and still merges successive mapping values; the widened participant and Memory contracts match every consumer, while the two other bundled participants remain valid narrower implementations. The Research Flow regression enables the real process appendix, writes a ledger, and proves the rendered scalar trail reaches both the message stamp and session record. I found no compatibility or merge-order regression.

Verification: the prior seven commits remain patch-identical under git range-diff; git diff --check github/main...HEAD passed; uv run pytest tests/test_participant_hook.py tests/test_agents_research_process_appendix.py tests/test_kernel_budget.py -x passed (110 tests); Ruff and format checks passed on all six files in the fix. Every completed GitHub check passes, with four unit jobs pending, and GitHub reports zero unresolved review threads. I covered the repository rules and context, diff and callers, history, backward compatibility, test strength, and architecture boundaries.

@ypflll
ypflll merged commit dbe259f into main Sep 20, 2026
23 checks passed
@ypflll
ypflll deleted the refactor/participant_contract_closeout branch September 20, 2026 03:36
LivXue pushed a commit that referenced this pull request Oct 12, 2026
…write (#495)

## Summary

#469 gave a bundled sub-agent nine verbs in place of six hook phases.
Reviewing it turned up a
list of things the contract said but did not yet do, and this PR closes
that list. No behaviour
change is intended: the point is that the same seam becomes something a
role can replace and
something a generated judgement can enter.

**1. The roles are named apart from the verbs, and `observe` is gone.**
`ActionModule.review`
and the participant's `review` were two different things a line apart.
The four delegating role
methods now say what they do -- they ask -- and the verbs keep their own
names, so `ask_review`
asks `review`. `observe` is removed: a verb with no return value decides
nothing and can only
keep state a participant is forbidden to keep. `outbound` says on its
own docstring that it is
the one verb with no role seat, and `StepView` gains `phase`, so a verb
asked at two moments
reads which one it is rather than inferring it from whichever other
field happens to be set
(`tools_ran` becomes a property derived from it, so the two cannot
disagree).

**2. The three verbs the seat was applying itself get a seat.**
`system_addendum`, `archive` and
`select_tools` were rendered straight onto the hook decision, so each
was the one judgement in
the set with nowhere to compose two participants, nowhere to vet what
came back, and nothing a
replacement role could decide. They join the other four:
`MemoryModule.ask_system_addendum` / `ask_archive` and
`CapabilityModule.ask_select_tools`. The
seat is not the point where narrowing takes effect -- `select` still is,
and the registry still
adjudicates every call -- so a participant narrows after the product has
spoken and never instead
of it.

**3. A participant answers with plain data.** `review`, `intake` and
`system_addendum` returned
host classes (`Verdict`, `Intake`) and `salvage` returned `Any`.
`Accept`, `Resample`, `End` and
`Intake` are now builders returning plain mappings, the host reads one
back with `read_verdict` /
`read_intake`, and an answer it cannot read is silence rather than a
refusal. That is what makes
the third invariant real and what lets a judgement compiled from source
answer the same verbs a
plugin does -- it can build a `dict`; it cannot build a host class.

**4. A dispatch's judgements are a participant.** `judge` was the one
verb a role answered by
reading the dispatch's own `checks` and `code` for itself. A Charter now
enters as a participant
like any other, `ActionModule.ask_judge` takes the list, and the merge
is a veto so the first
refusal decides. The participant list becomes the only route an external
judgement takes, which
is the thing step four needs: a judgement generated for one dispatch
joins the same list without
a second way in.

**5. The rename.** `AgentConduct` is `AgentParticipant`,
`contracts/agent_conduct.py` is
`contracts/participant.py`, `agent/hook/conduct.py` is
`agent/hook/participant.py`, and
`agent/harness/conducts.py` is `agent/harness/participants.py`.
"Conduct" was already taken in
this tree -- raven-code's Coding Conduct prompts are a different thing
-- and a reader hitting both
had no way to tell them apart. The rename was done symbol by symbol, not
by search and replace,
for exactly that reason.

**7. Review (LivXue, at 3a1bf07).** The `judge` disclosure had reached
two of its four homes;
the other two are `ActionModule.ask_judge` on the paper and
`DefaultAction.ask_judge`, and they
survived because they phrase the claim without naming any symbol the
earlier fix touched. The test
meant to pin that claim could not fail for the reason it was written --
it asserted the refusal
rather than the argument, and built a `ParticipantHook` it dropped --
and now asserts what the
registry hands the role, mutation-checked by wiring a participant
through. `compose_judge`
exploded a one-sentence `str` refusal into one refusal per character,
because `str` satisfies
`Sequence[str]` structurally; `charter_code.run_judge` coerced it where
this composition used to
live and the handling did not move with it. Two records corrected: the
type-only
`raven.contracts` -> `raven.agent` edge, the first in the tree and the
price of moving `Verdict`
and `Intake` off the paper, is now written beside the import contract it
bends rather than left to
be discovered; and the ledger entry for the 3,200 -> 3,410 bump is
restored to the names and count
it was written with. Not taken: reading `step.phase` at
`research_flow/flow.py:1000` -- the gate
chain's phase vocabulary and `StepView.phase` coincide on
`after_iteration` only, and that call
site wants `before_execute_tools`, which is the key `_phase` dispatches
on.

**6. What the paper was claiming and not doing.** Read back over the
seam afterwards rather than
over the diff. ``judge`` read as though a plugin's override were asked
ahead of a dispatch's own
rules; it is not, and cannot be as written, because the party that asks
the verb is
``ToolRegistry.execute``, which is not the hook chain and holds no
handle on this turn's
participants. The merge order is real; nothing puts a plugin in the
list. Wiring one through is a
turn-scoped register this change does not add, and it would start
refusing calls that run today,
so it is named on the paper and pinned by a test instead.
``max_iterations`` had lost its
docstring to the property inserted between the field and the string
documenting it. No executable
line moves in this commit: the module's AST, docstrings stripped, is
identical to its parent's.

CONTEXT.md's entry is rewritten to the end state, including the two
kinds of role method, the two
verbs that are exceptions, and the note that the Coding Conduct prompts
are unrelated.

One item from the same list is deliberately not here: a `planning.*`
block on the dispatch
parameters. It is not a contract closeout but a new model-facing field,
and it needs the
generator's schema and prompt to move with it.

Contract papers: `participant.py` and `harness.py` are both factory-loop
tier, so the
contract-tier digest and `CONTRACTS_VERSION` do not move.
`CONTRACTS_LINE_CEILING` 3,440 to
3,470 to 3,500 to 3,540 to 3,550 to 3,570, each bump with its own review
paragraph in
`tests/test_kernel_budget.py`. The chain was recomputed after rebasing
onto #410, which added
seven lines to `loop_hooks.py` inside the old headroom and left one
commit here with one line of
it; every "Measured at" figure in that file is the count at that commit.

## Type

- [ ] Fix
- [ ] Feature
- [ ] Docs
- [ ] CI / tooling
- [x] Refactor
- [ ] Other

## Verification

Gates, run at each of the seven commits:

```
uv run ruff check --no-cache raven evolver agents plugins-dist tests scripts
uv run ruff format --check --no-cache raven evolver agents plugins-dist tests scripts
uv run lint-imports
uv run pytest -q
npx commitlint --from origin/main --to HEAD --config commitlint.config.cjs
uv run python scripts/check_commit_messages.py origin/main..HEAD
uv run python scripts/check_source_language.py origin/main..HEAD
uv run python scripts/check_large_files.py origin/main..HEAD
```

Import contracts: 10 kept, 0 broken. The suite is green: the failures
this machine shows are the
same ones it shows on an unmodified `main` in the same worktree layout
(missing browser and
optional packages), and this branch adds none.

Behaviour preservation. The oracle is the hook seam itself. `main` and
this branch were run under
the same scripted provider, outputs normalised for nonces and temp
paths, then byte-compared.
Four scenarios, all identical:

- the five-plugin hook seam, every phase's decision and metadata per
iteration (23,840 bytes)
- a whole turn with no playbook: host turn, spawned sub-agent, tool
calls and refusals
- a playbook-on worker lane in process, including the charter refusals
- six window-shrink scenarios

This is the seam the change is about, and identical output across it is
what "no behaviour
change" means here -- it is not a claim about paths the four scenarios
do not reach.

Two claims had no assertion anywhere and now do. ``phase``, and the
``tools_ran`` derived from
it, are what three of the five bundled plugins branch on -- ppt on its
close, oncall on its
counter, research on which gate runs -- and a mistyped phase string at
one of the six call sites
would have changed what they do with a green suite. The read-only test
now also covers
``mode_overlay``, the one frozen field it skipped. All three assertions
were mutation-checked:
mislabelling a phase, deriving ``tools_ran`` from the wrong one, and
dropping the freeze each
turn exactly their own test red.

New tests, each written against a claim the contract now makes rather
than against the code:
a participant cannot write through the `StepView` it is shown; an answer
the host cannot read is
silence, not a refusal; a participant that answers with a mapping it
built itself is understood;
every verb with a seat goes through its role rather than around it,
asserted as a set so a new
verb that bypasses its role fails; two seats in one chain each keep one
addendum; two turns in
flight keep their own participants; a bound role decides what a
participant's verdict does; a
plugin's own judgement speaks before the dispatch's, and a silent
participant lets the next one
speak.

Real model, Sonnet 4.5 through OpenRouter, on this branch: a query
generates a playbook, two
sub-agents are instantiated and run, each writes its `summary.md` under
its own fence, and both
results are announced back to the originating session.

- [x] Relevant tests pass locally
- [x] Relevant lint / type checks pass locally
- [x] User-facing docs or screenshots are updated when needed

## Risk

For plugin authors: the six-phase `AgentHook` is untouched and a plugin
shipping one keeps
working. A plugin written against #469's `AgentConduct` needs the new
names and, if it answered
with `Verdict` or `Intake`, the builders instead -- the five bundled
plugins are ported here and
are the worked examples.

One verb narrowed, and silently for a plugin that used the width.
`salvage` was annotated
`Any | None` and `compose_salvage` returned whatever was not None; it is
now `str | None` and only
a non-empty string is taken. A third-party plugin that salvages with a
reply object rather than
text stops being heard rather than raising -- which is the third
invariant working as written, and
worth knowing before upgrading. The narrowing is deliberate: `salvage`
answers the text a turn
sends when it produced none, and `Any` was not a contract.

For anyone replacing a harness role: the four delegating methods are
renamed and four more are
added, so a replacement written against #469 needs them. `bind()` names
the missing member at
assembly rather than failing inside a turn.

Rollback is a revert of the merge commit. Nothing persisted on disk
changes shape.

- [x] Security impact considered
- [x] Backward compatibility considered
- [x] Rollback path is clear for risky changes

## Related Issues

Follows #468 and #469.

Generated with Claude Code (https://claude.com/claude-code)

---------

Co-authored-by: yao pengfei <yaopengfei@shanda.com>
Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants