Give the conductor's steps wire forms, and link rows and events - #69
Conversation
A host journals the conductor's commits and events and streams them to whatever draws the desk, so they now have pinned serde forms: internally tagged and snake_case, like Utterance and the driver's own payloads. Two links a desk needs were missing. A commit now names the conversation it belongs to -- a row said inside one, desk work lifted out of one, and the row that concludes it to its asker -- so an agent-to-agent exchange can be shown whole wherever its rows landed. And the events about a row now carry that row's sequence, with a handoff carrying the broadcast it came from, so a host attaches them to the row instead of inferring it from order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 5 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Changes requested Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Resolved this pass
Before merge
How this fits togetherflowchart LR
n0["Commit<br/>changed"]:::changed
n1["Event<br/>changed"]:::changed
n2["Note<br/>changed"]:::changed
n3["Refusal<br/>changed"]:::changed
n4["Turn<br/>changed"]:::changed
n5["...ersation_is_desk_work_and_a_dm_is_dropped<br/>changed<br/>1 finding"]:::flagged
n6["...ersation_is_open_is_refused_and_explained<br/>changed<br/>1 finding"]:::flagged
n7["...hat_runs_first_and_concludes_to_the_asker<br/>changed<br/>1 finding"]:::flagged
n8["wave"]:::impacted
n9["iter"]:::impacted
n10["...reply_in_a_conversation_is_not_its_answer"]:::impacted
n11["hive"]:::impacted
n12["begin_wave"]:::impacted
n13["Sequence"]:::impacted
n0 -->|uses| n13
n1 -->|uses| n3
n1 -->|uses| n13
n2 -->|uses| n13
n3 -->|uses| n13
n4 -->|uses| n13
n5 -->|calls| n8
n5 -->|calls| n11
n6 -->|calls| n8
n6 -->|calls| n11
n7 -->|uses| n0
n7 -->|calls| n8
n7 -->|calls| n11
n7 -->|calls| n13
n7 -->|tests| n13
n8 -->|calls| n9
n8 -->|tests| n9
n8 -->|calls| n12
n8 -->|tests| n12
n10 -->|uses| n0
n10 -->|uses| n2
n10 -->|calls| n8
n10 -->|calls| n9
n10 -->|tests| n9
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
|
Warning Review limit reached
This review includes 12 billable files and costs up to $3.00. Or wait 41 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (12)
Comment |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0890 · 1,057,957 in / 40,070 out · 72,162 cached (7%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 1,276 embedded
critique: $0.0456 · 556,637 in / 14,083 out · 31,180 cached (6%) · gpt-5.6-luna, deepseek-v4-flash
security: $0.0418 · 439,065 in / 8,034 out · 24,598 cached (6%) · gpt-5.6-luna
tests: $0.0007 · 27,972 in / 5,211 out · 0 cached (0%) · deepseek-v4-flash
description: $0.0006 · 18,586 in / 8,896 out · 1,024 cached (6%) · deepseek-v4-flash
| mod conversations; | ||
| mod desk; | ||
| mod door; | ||
| mod links; |
There was a problem hiding this comment.
Add the declared test modules before importing them
Rust resolves these declarations as links.rs/links/mod.rs and wire.rs/wire/mod.rs, but the repository contains neither module. As a result, compiling the test module fails with missing-file errors. Add the corresponding files, or remove these declarations until the modules are present.
[RULE] missing-module ·
There was a problem hiding this comment.
Both files are in this PR: crates/tinyhivemind-driver/src/conduct/test/links.rs and test/wire.rs are added by the same commit that declares them (the diff lists them as new files). The Rust job on this head compiles and runs them: 24 conductor tests pass, 13 of them in these two modules.
There was a problem hiding this comment.
Resolved — the reply explains why it is not a problem (advisory), as of 5d16143.
If this is wrong, reopen the conversation and say so; the finding will be re-raised on the next push if it still reproduces.
| seat: String, | ||
| /// Who took it. | ||
| to: Vec<String>, | ||
| /// The broadcast row. |
There was a problem hiding this comment.
Update every event constructor for the new required fields
Adding at as a required field to event variants changes every Event::Broadcast, Unplaced, CompletedByBroadcast, Handoff, Refused, Discharged, and Concluded construction site. The unchanged conductor code still constructs these variants using their previous field sets, so the crate will fail to compile until each caller supplies the corresponding sequence (or the field is made optional/defaulted).
[RULE] build-break ·
There was a problem hiding this comment.
Every construction site is updated in this commit: the seven sites in conduct/wave.rs (Broadcast, Unplaced, CompletedByBroadcast, Handoff, both Refused arms and Discharged, plus Concluded), and the example's matches take ... The Rust job on this head compiles the crate and the example, so no site was missed.
There was a problem hiding this comment.
Resolved — the reply explains why it is not a problem (advisory), as of 5d16143.
If this is wrong, reopen the conversation and say so; the finding will be re-raised on the next push if it still reproduces.
| pub(super) desk: Vec<(String, Utterance)>, | ||
| /// What desk turns said, and what thread turns said to the desk: | ||
| /// `(seat, utterance, the conversation it was lifted out of)`. | ||
| pub(super) desk: Vec<(String, Utterance, Option<Sequence>)>, |
There was a problem hiding this comment.
Update every desk queue producer for the new tuple shape
Changing desk from (String, Utterance) to (String, Utterance, Option<Sequence>) makes every existing push or assignment of the old two-element tuple type-check incorrectly. The unchanged producer(s) must be updated to include the conversation provenance, using None for ordinary desk turns and the originating root for lifted conversation work; otherwise the driver crate fails to compile.
[RULE] compile-error ·
There was a problem hiding this comment.
The one producer, Conductor::record in conduct/mod.rs, is updated in this commit: it pushes None for a desk turn and Some(root) for a broadcast or ask lifted out of a conversation, and conduct/test/links.rs asserts both. The Rust job on this head compiles.
There was a problem hiding this comment.
Resolved — the reply explains why it is not a problem (advisory), as of 5d16143.
If this is wrong, reopen the conversation and say so; the finding will be re-raised on the next push if it still reproduces.
| @@ -443,6 +459,7 @@ impl<'a, A: BoundAgent> Conductor<'a, A> { | |||
| let Some(child) = self.children.remove(&root) else { | |||
There was a problem hiding this comment.
Preserve the child until its conclusion commit succeeds
The conversation is removed before apply_committed can validate and apply the conclusion. If that fold fails, the method returns an error while the desk state remains unchanged, but the child and its pending conclusion are already gone, so the conversation cannot be retried or concluded later. Borrow or clone the child first, apply the transition successfully, and remove it only after the fold succeeds.
[RULE] state-preservation-on-error ·
There was a problem hiding this comment.
Fixed in the follow-up commit on this branch: the fold is applied first, and the conversation is removed only once it accepts the conclusion. A refused conclusion now leaves the conversation open and the next wave concludes it at a row the host gives properly; a_conclusion_the_fold_refuses_leaves_the_conversation_to_conclude_later covers it by reporting a sequence the episode already holds.
There was a problem hiding this comment.
Resolved — the review agent found this finding fixed in the new code, as of 5d16143.
If this is wrong, reopen the conversation and say so; the finding will be re-raised on the next push if it still reproduces.
The child was removed before its conclusion was applied, so a conclusion the fold refused left the desk unchanged and the conversation gone, with no way to conclude it later. The fold goes first now; a refused conclusion leaves the conversation open, and the next wave concludes it at a row the host gives properly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0316 · 331,250 in / 21,699 out · 31,188 cached (9%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 1,282 embedded
critique: $0.0138 · 126,446 in / 8,457 out · 14,280 cached (11%) · gpt-5.6-luna, deepseek-v4-flash
security: $0.0163 · 136,969 in / 5,586 out · 14,860 cached (11%) · gpt-5.6-luna
tests: $0.0006 · 29,223 in / 2,479 out · 1,024 cached (4%) · deepseek-v4-flash
description: $0.0004 · 19,841 in / 1,396 out · 1,024 cached (5%) · deepseek-v4-flash
| self.wave.event(Event::Handoff { | ||
| to: agent_id.clone(), | ||
| from: handoff.from.clone(), | ||
| origin: handoff.origin, |
There was a problem hiding this comment.
Define HandOff::origin before referencing it
This line accesses handoff.origin, but the HandOff definition has not been updated to provide that field in this change. That leaves the driver with an unknown-field compilation error. Add the origin field to HandOff and populate it at every construction site, or remove this access if provenance is not part of the handoff contract.
[RULE] missing-field ·
| assert!(!conductor.finished(), "the conversation is still open"); | ||
|
|
||
| // The next wave concludes it, at a row the host gives properly. | ||
| let later = wave(&mut conductor, &journal, &[]).expect("wave"); |
There was a problem hiding this comment.
Preserve the child until its conclusion commit succeeds
The failed conductor.committed(sequence) consumes the outstanding commit, and the conductor still removes the conversation before the driver's fold can succeed. As a result, this follow-up wave has no child left to retry or conclude, so the new regression test fails (or the conversation is permanently lost). Apply the conclusion first and remove the child only after the transition succeeds, preserving the outstanding conversation when the fold rejects the supplied sequence.
[RULE] state-preservation-on-error ·
| pub(super) desk: Vec<(String, Utterance)>, | ||
| /// What desk turns said, and what thread turns said to the desk: | ||
| /// `(seat, utterance, the conversation it was lifted out of)`. | ||
| pub(super) desk: Vec<(String, Utterance, Option<Sequence>)>, |
There was a problem hiding this comment.
Update every desk queue producer for the new tuple shape
The desk queue now requires (String, Utterance, Option<Sequence>), but the existing producers in this file still construct the old two-element tuples. The driver will fail to compile until every producer supplies the conversation provenance, using None for ordinary desk work and the originating root for lifted conversation work.
[RULE] compile-error ·
1 participant
Summary
The conductor hands its host steps: turns to run, notes and commits to append, events to show. A host like OpenCompany journals the commits and streams the events to the UI that draws the desk, so these types need wire forms, and two links the UI needs were missing.
Turn,Channel,Note,Commit,Refusal,EventandStepnow derive serde, internally tagged andsnake_caselikeUtteranceand the driver's own payloads. AStepis tagged bystepwith its fields beside the tag; anEvent,RefusalandChannelbykind.Commit::conversationis the ask row the conversation is rooted at. It is set for rows said inside a conversation, for desk work a seat lifted out of one (a broadcast or an ask made while talking, which lands on the desk with no thread), and for the row that concludes it to its asker. A host shows an agent-to-agent exchange whole by taking the ask row and every row with its sequence asconversation, wherever they landed.Broadcast,Unplaced,CompletedByBroadcast,Refused,DischargedandConcludedcarryat, the sequence the host gave the row they are about.Handoffcarriesorigin, the broadcast row it came from. A refused row is already on the journal, so its event is what marks it refused.Related issue
None. First of the changes laid out for integrating the conductor into OpenCompany; follows #68.
API or behavior changes
Additive to the public surface, but breaking for exhaustive matches:
Commit::conversation.aton sixEventvariants andoriginonEvent::Handoff. A host that destructures these variants without..must add it; theconductedexample is updated.Kinda commit carries is serialized aspurposeso a commit survives the wire whole; it stays opaque to a host.No change to which steps the conductor emits or in what order.
Validation
Commands actually run, with their outcome:
cargo fmt --all -- --check-- cleancargo clippy --all-targets --all-features -- -D warnings-- cleancargo build --all-targets --all-features-- okcargo test --all-features-- all green; plaincargo testtoo.github/scripts/assert-pure.shandassert-openhuman-pin.sh-- clean.github/scripts/check-file-coverage.sh 90-- every changed file 95% or aboveRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features-- okexamples/openhuman: clippy,cargo test -- --skip security::preflight(49 passed; the two Docker preflight tests hang without Docker), and the offline proofs on both runners and both desksTests
conduct/test/wire.rs: the exact JSON of a turn, a note, a commit and an event; every event and refusal variant round-trips alone and inside a step; missing required fields are rejected.conduct/test/links.rs: every row of a conversation carries its root, including the lifted broadcast and the conclusion, and a desk row and the ask itself carry none;Broadcast,CompletedByBroadcast,Unplaced,Discharged,RefusedandConcludedname the sequence the host gave their row, andHandoffits origin.Documentation
stepsand the conduct README describe the wire forms and how a host reads a conversation and an event's row back.Checklist
#[allow(...)],#[ignore], or relaxed lints.envcontents in the diff or the description🤖 Generated with Claude Code