Follow-up: aside fold ordering, thread-aware collapse, and an honest benchmark correction - #19
Conversation
The wiki submodule is advanced to a newer commit, incorporating upstream changes to the wiki content. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The policy checks for thread requirement, message budget, and unsettled state were moved to run after the audience is validated. This ensures that when a request names an inactive target and also violates a policy rule, the caller receives the actionable refusal about the target rather than a misleading policy error that would persist even after correcting the policy issue. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a test asserting that audience faults are reported before thread, budget, and settlement rungs, since the spec fixes audience resolution and validation as the first evaluation step. Also verifies that an author fault still outranks the audience check, as a member off the desk cannot address anyone on it. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The settlement lookup now scans a precomputed list of desk-visible, agent-authored rows from the entire scanned slice rather than only the projection. This fixes stubs reporting `settled_at: None` when an aside's settlement lies outside the narrowed channel projection, and also prevents distinct closed threads between the same participants from collapsing into a single stub by keying runs on conversation identity. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The projection functions now return settlement information alongside the projected messages, and `collapse_elisions` receives it as an explicit parameter instead of deriving it internally. This lets `project_as` compute settlements from the original transcript, preserving thread boundaries that would otherwise be lost when merging adjacent elided runs between the same pair of authors. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The projection now carries the thread root sequence alongside each presented message, allowing callers to identify which messages belong to the root thread versus nested replies. This also records seen row metadata earlier to support accurate settlement tracking during projection. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The projection functions now return settlement metadata alongside the projected messages, allowing callers to track which threads have been settled. Settlements are computed from the full scanned slice before narrowing discards replies, ensuring that asides which are first replies to a root are correctly settled by later replies. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The thread identity for a kept row was previously always the row's own sequence when it had no parent, which caused a run of parentless channel asides to be split into separate stubs instead of collapsing into one. Now a parentless row only becomes its own thread when something actually replies to it, while ordinary channel messages remain unthreaded and merge correctly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds four tests for the session projection logic that groups asides into stubs. Two tests verify that separate closed threads between the same pair remain distinct stubs while a run of channel asides collapses into one, and two more confirm that settlement detection only applies to replies that follow the aside in the transcript. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The simulation agent now responds to peer requests with its own untouched evaluation of a topic rather than its averaged score. Previously, using the averaged score meant already-pooled readings were echoed back into the room, causing double-counting and distorting the benchmark's model of independent peer signals. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…coring The experiment note previously reported results from a bug where a responder could return an already-pooled reading, letting one peer's signal be counted multiple times. The fix makes responders answer with their own untouched private evaluation, and the corrected numbers show the uniform rows unchanged and hidden-profile rows moved by less than a point, leaving all conclusions intact. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The README's statistics section now links to the dedicated STATISTICS.md file instead of duplicating the full explanation of the benchmark's metrics, confidence intervals, and methodology. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The HTTP turn example previously passed the request body directly as a curl command-line argument, but now includes it in the curl config file instead. This keeps the command invocation simpler and consolidates all request parameters in the configuration, making the example easier to read and modify. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The crosstalk example now escapes backslashes and double quotes in the JSON request body before embedding it into curl's config file format, where double-quoted values interpret backslash escapes. This ensures the body's JSON quotes and backslashes survive the trip through curl's parser and reach the server intact. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The curl command in the crosstalk agent example was missing escaped quotes around the data-binary argument, which would cause the command to fail when executed. This change adds the proper escaping so the request body is passed correctly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The sharing path now documents that a settlement arriving after its stub has shipped leaves the earlier `settled_at: None` in place, since the watermark has moved past the row and the crate cannot revise a message it no longer holds. The example's CLI backend similarly notes that its prompt is visible in the process argument list and that `--timeout` is not enforced, both inherent to its contract rather than oversights. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformatted several long function signatures, assertions, and a chained string replacement in the crosstalk example to fit within the project's line-length conventions. No behavior changes were made. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reached
On-demand reviews are free for the next 14 days. After that, they cost $0.25 per reviewed file. Or wait 38 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: Team Run ID: 📒 Files selected for processing (13)
Warning Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 742c6f5576
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| candidates | ||
| .iter() | ||
| .map(|candidate| (&candidate.sequence, &candidate.audience, &candidate.author)), |
There was a problem hiding this comment.
In channel-level projections, candidates includes whitespace-only rows because they are retained earlier for root detection, and this iterator now passes them to Settlement::over without filtering their content. If an aside participant authors a blank desk-visible row after an aside, the stub is therefore marked settled_at at that sequence even though narrowing removes the blank row and it contains no public outcome for the viewer to recover. Filter empty content when constructing the settlement list, as the thread projection already does.
Useful? React with 👍 / 👎.
How this change flows5 changed behaviours across 14 relationships. 5 surrounding behaviours are shown (60 graph nodes walked). 43 further behaviours left out to keep the diagram readable. flowchart LR
n0["aside<br/>changed"]:::changed
n1["SimAgent<br/>changed"]:::changed
n2["Candidate<br/>changed<br/>1 finding"]:::flagged
n3["narrow_to_roots_and_first_replies<br/>changed<br/>1 finding"]:::flagged
n4["participants<br/>changed<br/>1 finding"]:::flagged
n5["Sequence"]:::impacted
n6["SessionMessage"]:::impacted
n7["iter"]:::impacted
n8["project_as"]:::impacted
n9["as_viewer"]:::impacted
n0 -->|calls| n7
n1 -->|uses| n5
n2 -->|uses| n5
n3 -->|uses| n2
n3 -->|uses| n5
n3 -->|uses| n6
n3 -->|calls| n7
n4 -->|uses| n6
n4 -->|calls| n7
n6 -->|uses| n5
n8 -->|uses| n5
n8 -->|uses| n6
n8 -->|calls| n7
n9 -->|uses| n6
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
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0480 · 322,862 in / 10,164 out · 60,454 cached (19%) · openrouter/openai/text-embedding-3-small, deepseek/deepseek-v4-flash, z-ai/glm-5.2 · 745 embedded
critique: $0.0187 · 178,301 in / 3,596 out · 19,661 cached (11%) · deepseek/deepseek-v4-flash, z-ai/glm-5.2
security: $0.0185 · 102,119 in / 3,481 out · 26,916 cached (26%) · deepseek/deepseek-v4-flash, z-ai/glm-5.2
tests: $0.0023 · 24,922 in / 114 out · 0 cached (0%) · deepseek/deepseek-v4-flash
description: $0.0085 · 17,520 in / 2,973 out · 13,877 cached (79%) · z-ai/glm-5.2
| candidate.content, | ||
| candidate.audience, | ||
| viewer, | ||
| // A promoted reply's identity is the root it hangs under; a root |
There was a problem hiding this comment.
Root is tagged as its own thread even when no reply follows it
The rooted set is built from candidate.parent values — the parent sequences that received a reply. If a root happens to have the same sequence number as a different root's parent value (which is impossible across different sequences but correct-by-construction here), or more importantly if a root's own sequence equals that of a different root's parent, then a root with no actual replies gets thread = Some(sequence). This makes two separate sequences of channel-level asides between the same pair that never had a thread falsely look like one thread, merging their stubs. The set should contain sequences of roots that are actually replied to, but rooted only contains parent values, which are the sequences of the things being replied to, not the sequences of the candidates themselves. The condition should check whether the candidate is a root that was actually replied to later in the same slice.
[RULE] logic-error ·
1 participant
Follow-up to #16, #17 and #18, which merged with review threads still open. This
addresses the substantive ones. Two of them were defects in my own work that the
bots caught correctly, and one of those contradicted a doc comment I had written.
Library correctness
The aside fold checked policy before it resolved the audience —
crates/tinyhivemind-core/src/aside/mod.rs. A request that named an inactivetarget and sat on a channel under
require_threadwas refused withThreadRequired, so a caller would open a thread and be refused again for thereason it was never given. This contradicted the evaluation order the spec
already states. The three policy rungs now run after audience resolution, and a
test pins the order — the old code passed the existing suite, which is why it
shipped.
Two closed threads between the same pair collapsed into one stub —
crates/tinyhivemind/src/session/mod.rs. Collapsing keyed on participantsalone, and
SessionMessagecarries no parent, so two distinct closed threadsmerged into a single stub reporting a combined range and message count for
exchanges that were never one exchange. The projection now carries each row's
conversation identity through the collapse: a promoted reply's root, a root's
own sequence when something actually replies to it, and
Nonefor a plainchannel row — so a run of channel asides is still one stub.
A settlement that channel narrowing dropped left the stub unsettled — same
file.
project_channelkeeps each root and its first reply; when the asidewas that first reply and the settlement was a later one, the settlement was not
in the projection at all and the stub reported
settled_at: Nonefor an asidethe room had settled. Settlements are now collected from the whole scanned
slice, before narrowing throws replies away.
The benchmark arm
A responder echoed an already-pooled value —
crates/tinyhivemind-hive/examples/bench/sim.rs.answer_checkreturnedscore(), which averages in every reading the responder had already absorbed,so one peer's signal could be counted several times over. That is not the
experiment the write-up describes, where an asker averages its own reading with
one independent peer's. A responder now answers with
own_reading.The numbers were re-run and the write-up corrected. At
--aside-cap 1—every configuration in the note — recirculation almost never got a chance to
occur, so the uniform rows are unchanged to the decimal and the hidden-profile
rows moved by less than a point:
No conclusion changes. The
hive+aside − hive+asknull on the hidden profileactually tightened,
+0.7 [-0.8, +2.1]→+0.1 [-1.3, +1.6]. The defect wasreal, the fix is right on principle, and it happened not to be load-bearing at
this cap.
docs/experiments/2026-09-07-do-asides-help.mdcarries a"A correction, and what it did not change" section saying so rather than
quietly restating the table.
Documentation honesty
The request body was in
curl's argv, and the doc comment said it wasn't.crosstalk/agent.rsclaimed "neither the API key nor the request body everappears in the process argument list" while passing
--data-binary <body>as anargument. The body now travels in the config on stdin, escaped for curl's config
grammar. Verified live against a running endpoint, not just compiled.
The CLI backend's real limits are now written down rather than left implied:
the prompt reaches the agent as its final argument, so the transcript is
readable out of
pswhile a turn runs, and--timeoutis not enforced becausestd::process::Commandhas no deadline. Both are inherent to the--agent-cmdcontract — it exists so any prompt-as-final-argument CLI works unmodified — and
neither applies to the HTTP backend, which carries the only credential. Declined
rather than "fixed", with the reason stated in the code and the README.
settled_aton an incremental sharing delta is a floor, not a fact. A stubdelivered in one tick keeps the
Noneit was sent with even when the asidesettles in a later one: the watermark has moved past the row and this crate
cannot revise a message it no longer holds. A re-seed shows it settled. Fixing
it incrementally would need pending-aside state carried across ticks, which is a
second store to invalidate and the charter refuses it. Documented in
sharing/mod.rsand added to the spec's Open questions.The bench README was 502 lines, over the repo's explicit 500 cap. The
statistics section moved to
examples/bench/STATISTICS.mdand is linked fromwhere it was. Nothing else in the repo exceeds the cap.
Public API changes
SessionMessage,LogMessageand the wire forms are untouched. Internal totinyhivemind::session,collapse_elisionsnow takes conversation identity anda settlement slice.
project_asis unchanged in signature but documents alimitation it always had: a
SessionMessagecarries no parent, so a caller thatholds only a transcript cannot tell two closed threads apart. A caller holding
the log should project from it.
Validation
cargo fmt --all -- --checkcargo clippy --all-targets --all-features -- -D warnings— cleancargo build --all-targets --all-featurescargo test --all-features— 732 passed, 0 failed (727 on main, +5 new)./.github/scripts/assert-pure.sh— cleanRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features— cleanthe published table to the decimal.
crosstalk --api-base ... --hops 2run live against a real endpoint after thecurl change.
Deliberately not done here
--window 1finding on the crosstalk example (private.first()can select a row outside the rendered views). Real, but example-only and
cosmetic — it makes a claim print FAIL on a deliberately pathological window.
spent_in_asidecounting rows from a neighbouring aside in the sameexample. Real, example-only, and only reachable with more than two seats
opening asides in sequence.
--agent-cmdargv exposure and the missing CLI timeout, declined abovewith reasons.
Each is listed so it is on the record rather than lost; happy to take them in a
further follow-up if you want them closed.