Skip to content

perf(loop): write finalized assistant reply via one-shot append path - #5471

Closed
serrrfirat wants to merge 1 commit into
mainfrom
claude/finalized-append-wiring
Closed

serrrfirat wants to merge 1 commit into
mainfrom
claude/finalized-append-wiring

Conversation

@serrrfirat

Copy link
Copy Markdown
Collaborator

Summary

  • Wires the live turn-completion transcript path (ThreadBackedLoopTranscriptPort::finalize_assistant_message in ironclaw_loop_support) onto the append_finalized_assistant_message primitive landed in perf(storage): row-native sequence primitive + thread/turn append paths #5455, replacing the former append-draft-then-CAS-finalize two-write sequence with a single call.
  • On the filesystem backend with no streamed draft, this takes the append-only fast path (one log append + sequence index, no per-message file write or read-modify-write CAS rewrite) — the storage win perf(storage): row-native sequence primitive + thread/turn append paths #5455 measured but did not yet realize in the live loop (nothing called append_finalized_assistant_message outside the stress harness).
  • Streaming stays correct: when the loop streamed a draft first (begin_assistant_draft), the one-shot resolves that draft by turn_run_id and finalizes it in place.
  • Concurrency convergence is preserved: a finalize that loses a concurrent race converges onto the winner's message via finalized_assistant_message_by_run instead of failing the turn (matching the prior already_finalized_matching_reply fallback). A divergent re-run (already-finalized reply, different content) still surfaces a transcript write failure.

Why

#5455 landed the reserve_sequence primitive and the append-only append_finalized_assistant_message path, but the live engine never called the latter — so the headline storage win was a capability, not something production realized. This is the adoption follow-up: it routes the single live assistant-finalize call site onto the append path so real turns get the collapsed write. Kept deliberately separate from #5455 so the behavior change (and its append-only read dependency) gets its own review.

Change Type

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • CI/Infrastructure
  • Security
  • Dependencies

(Performance — realizes the #5455 storage win in the live loop.)

Linked Issue

Follow-up to #5455 (merged). Builds on the append_finalized_assistant_message primitive added there.

Validation

Built and tested against the affected crate (rustc 1.96, crate built in isolation):

  • cargo test -p ironclaw_loop_support — all pass (454 across all targets), including the new caller-level cases and the pre-existing concurrency-convergence test.
  • cargo clippy -p ironclaw_loop_support --all-features --tests — clean.
  • rustfmt on the changed files.
  • Full-workspace fmt/clippy/test --features integration — not run; the workspace has a github.com git dependency (monty, via ironclaw_engine) and git-over-HTTPS to github.com is denied by the environment's egress policy, so cargo cannot resolve the full workspace. The affected crate was built/tested in isolation. Please run the standard quality gate in CI.
  • New/updated tests (drive the port, not the helper):
    • transcript_port_finalize_assistant_reply_is_idempotent_for_one_run — double finalize → one finalized row, same ref.
    • transcript_port_finalize_finalizes_existing_streamed_draft_in_place — begin_assistant_draft + update_assistant_draft then finalize → finalizes the draft in place, one row.
    • transcript_port_finalize_is_idempotent_under_concurrent_duplicate_calls (pre-existing) — still green via the convergence fallback.

Security Impact

None. No change to permissions, network, secrets, file access, or sandbox policy. Same SessionThreadService trait surface; this only changes which method the transcript adapter calls.

Reborn Trust-Boundary Checklist

  • No new policy/evidence/trust-bearing types.
  • No untrusted content enters prompts; the reply content path is unchanged.
  • No hashes added.
  • Status/runtime variants: none changed. Finalize still produces a Finalized assistant message.
  • Security/durability serde(default): none added.
  • Bounds/overflow: none introduced; sequence allocation is owned by the perf(storage): row-native sequence primitive + thread/turn append paths #5455 primitive.
  • Driver-visible errors unchanged: a failed finalize still maps to TranscriptWriteFailed; the divergent-content guard preserves the prior contract.
  • Backend/host names unchanged.

Database Impact

None. No new migration; relies on #5455's root_filesystem_sequences table and append-event log, both already on main.

Blast Radius

ironclaw_loop_support transcript port only (finalize_assistant_message impl + one private convergence helper; removed the now-redundant already_finalized_matching_reply). No public API change — downstream callers (ironclaw_reborn loop driver host, hook middleware) see the same trait surface and the same observable outcome (one finalized assistant message, one AssistantReplyFinalized milestone).

Rollback Plan

Revert this commit — it restores the prior append-draft-then-finalize write path. Note: finalized assistant replies written while this is live are stored append-only (no per-message file). Reverting this PR is safe because main (post-#5455) still merges the append log on reads. A deeper rollback past #5455 would make those append-only replies invisible to the older read path (data retained, not displayed); that is the only scenario where this needs care.

Review Follow-Through


Review track: C (runtime)

🤖 Generated with Claude Code


Generated by Claude Code

Wire the live turn-completion transcript path
(`ThreadBackedLoopTranscriptPort::finalize_assistant_message`) onto the
`append_finalized_assistant_message` primitive landed in #5455, replacing
the former append-draft-then-CAS-finalize two-write sequence with a single
call.

On the filesystem backend with no streamed draft, this takes the
append-only fast path (one log append + sequence index, no per-message
file write or read-modify-write CAS rewrite) — the storage win #5455
measured but did not yet realize in the live loop. When the loop streamed
a draft first (`begin_assistant_draft`), the one-shot resolves that draft
by `turn_run_id` and finalizes it in place, so streaming stays correct.

Concurrency: the former path's idempotent-under-concurrent-duplicate
convergence (a resumed/retried turn racing the original) is preserved.
On a finalize that loses the race, we converge onto the winner's message
via `finalized_assistant_message_by_run` rather than failing the turn,
matching the prior `already_finalized_matching_reply` fallback. A
divergent re-run (already-finalized reply with different content) still
surfaces a transcript write failure.

Tests (drive the port, not the helper):
- idempotent double-finalize → one finalized row, same ref
- streamed begin+update then finalize → finalizes draft in place, one row
- pre-existing concurrent-duplicate convergence test still green

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PQzj2447pq2BUfgY3BUSJb
@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5471 June 30, 2026 21:49 Destroyed
@github-actions github-actions Bot added size: M 50-199 changed lines risk: low Changes to docs, tests, or low-risk modules contributor: core 20+ merged PRs labels Jun 30, 2026
@coderabbitai

coderabbitai Bot commented Jun 30, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Assistant message finalization is now more reliable and consistent, even when called multiple times for the same turn.
    • Previously started assistant drafts are finalized in place, preserving the same message reference.
  • Bug Fixes

    • Improved handling of concurrent finalization so duplicate assistant replies are avoided.
    • Ensured finalized assistant content must still match the expected reply, reducing mismatched transcript entries.

Walkthrough

The finalize path in ironclaw_loop_support's LoopTranscriptPort was rewritten from a two-step draft-then-finalize sequence into a single-shot finalize keyed on turn_run_id. On append failure it converges via a new helper that matches finalized content for the run. A removed helper and updated imports support this. New tests cover idempotency and finalize-after-draft behavior.

Changes

Assistant Finalize Rewrite

Layer / File(s) Summary
One-shot finalize and convergence logic
crates/ironclaw_loop_support/src/lib.rs
finalize_assistant_message now appends directly via append_finalized_assistant_message; on failure it calls new helper finalized_reply_for_run_matching to find a concurrently finalized match by content, erroring on content mismatch. Old already_finalized_matching_reply removed; imports updated to drop unused MessageStatus and add the new request types.
Finalize idempotency and draft-finalization tests
crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs
New tests assert duplicate finalize calls return the same message ref without duplicate rows, and that finalizing after a streamed draft updates the draft in place rather than creating a new message.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant LoopTranscriptPort
  participant ThreadsStore

  Caller->>LoopTranscriptPort: finalize_assistant_message(turn_run_id, reply_content)
  LoopTranscriptPort->>ThreadsStore: append_finalized_assistant_message
  alt append succeeds
    ThreadsStore-->>LoopTranscriptPort: new finalized message ref
  else append fails (concurrent write)
    LoopTranscriptPort->>ThreadsStore: finalized_reply_for_run_matching(turn_run_id, reply_content)
    ThreadsStore-->>LoopTranscriptPort: finalized message if content matches, else none
    alt content mismatch or not found
      LoopTranscriptPort-->>Caller: transcript write failure
    end
  end
  LoopTranscriptPort-->>Caller: assistant_reply_finalized milestone
Loading

Review note: confirm the append-failure path is actually distinguishable from a generic store error before treating it as "concurrent winner" — conflating the two violates fail-closed error handling expected for storage writes; no panics introduced, but silent content-mismatch-to-error conversion needs a test asserting the exact error variant, not just message content.

🎯 3 (Moderate) | ⏱️ ~25 minutes

A draft once danced in two careful steps,
now one bold write and the convergence it keeps.
Same run, same words — no duplicate ghost,
🦀 finalized once, idempotent host.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is Conventional Commits-style and clearly summarizes the main change: one-shot finalization of assistant replies.
Description check ✅ Passed All required sections are present and substantially filled, including summary, linked issue, validation, security, DB, rollback, and review notes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the assistant message finalization flow in ironclaw_loop_support by collapsing the two-step append-draft-then-finalize sequence into a single append_finalized_assistant_message call, and adds corresponding integration tests to verify idempotency and in-place finalization. The reviewer suggests simplifying the concurrency fallback logic by retrieving the existing finalized message regardless of content matching in the helper function, which avoids logging misleading database constraint errors during divergent re-runs and cleanly delegates the content comparison to the caller.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +638 to +644
match self
.finalized_reply_for_run_matching(&reply_content)
.await?
{
Some(existing) => existing,
None => return Err(transcript_write_error(append_error)),
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If append_finalized_assistant_message fails (e.g., due to a unique constraint violation on turn_run_id because another concurrent finalize won the race), but the already finalized message has different content (a divergent re-run), the current implementation of finalized_reply_for_run_matching will filter it out and return None.

As a result, the caller will fall back to returning transcript_write_error(append_error), which logs a warning with the raw database unique constraint violation details. This is noisy and misleading because a divergent re-run is a known business logic error that should be handled cleanly.

By fetching the existing finalized message regardless of content (removing the filter from the helper), and letting the subsequent content-comparison check (finalized.content.as_deref() != Some(reply_content.as_str())) handle it, we cleanly return the expected TranscriptWriteFailed error for divergent content without logging the database error as a warning/error.

Suggested change
match self
.finalized_reply_for_run_matching(&reply_content)
.await?
{
Some(existing) => existing,
None => return Err(transcript_write_error(append_error)),
}
match self
.finalized_reply_for_run()
.await?
{
Some(existing) => existing,
None => return Err(transcript_write_error(append_error)),
}

Comment on lines +727 to +741
async fn finalized_reply_for_run_matching(
&self,
reply_content: &str,
) -> Result<Option<ThreadMessageRecord>, AgentLoopHostError> {
let existing = self
.thread_service
.finalized_assistant_message_by_run(FinalizedAssistantMessageByRunRequest {
scope: self.thread_scope.clone(),
thread_id: self.run_context.thread_id.clone(),
turn_run_id: self.run_context.run_id.to_string(),
})
.await
.map_err(transcript_write_error)?;
Ok(existing.filter(|message| message.content.as_deref() == Some(reply_content)))
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Simplify this helper to return the existing finalized message regardless of content. This allows the caller to cleanly handle divergent content checks and avoid logging misleading database unique constraint warnings under concurrent races.

    async fn finalized_reply_for_run(
        &self,
    ) -> Result<Option<ThreadMessageRecord>, AgentLoopHostError> {
        self.thread_service
            .finalized_assistant_message_by_run(FinalizedAssistantMessageByRunRequest {
                scope: self.thread_scope.clone(),
                thread_id: self.run_context.thread_id.clone(),
                turn_run_id: self.run_context.run_id.to_string(),
            })
            .await
            .map_err(transcript_write_error)
    }

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ironclaw_loop_support/src/lib.rs`:
- Around line 612-619: Soften the adapter comment in the one-shot finalize flow
so it only describes the thread-service contract, not filesystem-backend
implementation guarantees. In the comment near the finalize logic in
ironclaw_loop_support::lib, remove claims like “one log append + sequence index”
and “no per-message file rewrite,” and keep the wording focused on
`turn_run_id`, `begin_assistant_draft`, and the idempotent finalize behavior. If
backend details are important, move them to the backend implementation
tests/docs instead of this adapter comment.

In `@crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs`:
- Around line 2255-2373: The finalize tests in
transcript_port_finalize_assistant_reply_is_idempotent_for_one_run and
transcript_port_finalize_finalizes_existing_streamed_draft_in_place only cover
duplicate-content convergence; add a new caller-level test that finalizing the
same run with different assistant content returns TranscriptWriteFailed. Reuse
ThreadFixture and ThreadBackedLoopTranscriptPort, invoke
finalize_assistant_message twice with mismatched content for the same
turn_run_id, and assert history stays unchanged with no extra assistant row.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b8623700-991b-47e9-874c-c55c73c54ed1

📥 Commits

Reviewing files that changed from the base of the PR and between b6afc68 and 211783f.

📒 Files selected for processing (2)
  • crates/ironclaw_loop_support/src/lib.rs
  • crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs

Comment on lines +612 to +619
// One-shot finalize keyed on `turn_run_id`. This collapses the former
// append-draft-then-CAS-finalize two-write sequence into a single
// call: on the filesystem backend with no prior draft it takes the
// append-only fast path (one log append + sequence index, no
// per-message file rewrite); if the loop streamed a draft first
// (`begin_assistant_draft`), it resolves that draft by run and
// finalizes it in place. It is idempotent for a resumed/retried turn —
// a second call returns the already-finalized message.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Soften the filesystem-backend guarantee in this adapter comment.

This module does not enforce “one log append + sequence index” or “no per-message file rewrite”; keep the comment to the one-shot thread-service contract, or move backend internals to the backend test/docs.

Suggested wording
-        // One-shot finalize keyed on `turn_run_id`. This collapses the former
-        // append-draft-then-CAS-finalize two-write sequence into a single
-        // call: on the filesystem backend with no prior draft it takes the
-        // append-only fast path (one log append + sequence index, no
-        // per-message file rewrite); if the loop streamed a draft first
-        // (`begin_assistant_draft`), it resolves that draft by run and
-        // finalizes it in place. It is idempotent for a resumed/retried turn —
-        // a second call returns the already-finalized message.
+        // One-shot finalize keyed on `turn_run_id`. This collapses the former
+        // append-draft-then-CAS-finalize sequence into a single thread-service
+        // call. If the loop streamed a draft first (`begin_assistant_draft`),
+        // the thread service resolves that draft by run and finalizes it in
+        // place. It is idempotent for a resumed/retried turn: a second call
+        // returns the already-finalized message.

As per coding guidelines, “Comments that promise guarantees across layers must either be enforced by code/tests or softened to describe intent.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// One-shot finalize keyed on `turn_run_id`. This collapses the former
// append-draft-then-CAS-finalize two-write sequence into a single
// call: on the filesystem backend with no prior draft it takes the
// append-only fast path (one log append + sequence index, no
// per-message file rewrite); if the loop streamed a draft first
// (`begin_assistant_draft`), it resolves that draft by run and
// finalizes it in place. It is idempotent for a resumed/retried turn —
// a second call returns the already-finalized message.
// One-shot finalize keyed on `turn_run_id`. This collapses the former
// append-draft-then-CAS-finalize sequence into a single thread-service
// call. If the loop streamed a draft first (`begin_assistant_draft`),
// the thread service resolves that draft by run and finalizes it in
// place. It is idempotent for a resumed/retried turn: a second call
// returns the already-finalized message.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_loop_support/src/lib.rs` around lines 612 - 619, Soften the
adapter comment in the one-shot finalize flow so it only describes the
thread-service contract, not filesystem-backend implementation guarantees. In
the comment near the finalize logic in ironclaw_loop_support::lib, remove claims
like “one log append + sequence index” and “no per-message file rewrite,” and
keep the wording focused on `turn_run_id`, `begin_assistant_draft`, and the
idempotent finalize behavior. If backend details are important, move them to the
backend implementation tests/docs instead of this adapter comment.

Source: Coding guidelines

Comment on lines +2255 to +2373
/// Finalizing twice for the same run (e.g. a resumed/retried turn) must be
/// idempotent: one finalized assistant row, same message ref both times. The
/// one-shot `append_finalized_assistant_message` path the port now uses keys on
/// `turn_run_id`, so the second call resolves the existing finalized message
/// rather than appending a duplicate.
#[tokio::test]
async fn transcript_port_finalize_assistant_reply_is_idempotent_for_one_run() {
let fixture = ThreadFixture::new().await;
let adapter = ThreadBackedLoopTranscriptPort::new(
Arc::clone(&fixture.thread_service),
fixture.thread_scope.clone(),
fixture.run_context.clone(),
);

let first = adapter
.finalize_assistant_message(FinalizeAssistantMessage {
reply: AssistantReply {
content: "final answer".to_string(),
},
})
.await
.unwrap();
let second = adapter
.finalize_assistant_message(FinalizeAssistantMessage {
reply: AssistantReply {
content: "final answer".to_string(),
},
})
.await
.unwrap();

assert_eq!(first.as_str(), second.as_str());
let history = fixture
.thread_service
.list_thread_history(ThreadHistoryRequest {
scope: fixture.thread_scope.clone(),
thread_id: fixture.thread_id.clone(),
})
.await
.unwrap();
let assistant_rows: Vec<_> = history
.messages
.iter()
.filter(|message| message.kind == MessageKind::Assistant)
.collect();
assert_eq!(
assistant_rows.len(),
1,
"double finalize must not materialize a second assistant row"
);
assert_eq!(assistant_rows[0].status, MessageStatus::Finalized);
assert_eq!(assistant_rows[0].content.as_deref(), Some("final answer"));
}

/// When the loop streamed a draft first (`begin_assistant_draft` +
/// `update_assistant_draft`), finalizing must finalize that existing draft IN
/// PLACE — same message id, final content — not append a second message. The
/// one-shot path resolves the streamed draft by `turn_run_id` and finalizes it.
#[tokio::test]
async fn transcript_port_finalize_finalizes_existing_streamed_draft_in_place() {
let fixture = ThreadFixture::new().await;
let adapter = ThreadBackedLoopTranscriptPort::new(
Arc::clone(&fixture.thread_service),
fixture.thread_scope.clone(),
fixture.run_context.clone(),
);

let draft_ref = adapter
.begin_assistant_draft(BeginAssistantDraft {
reply: AssistantReply {
content: "partial".to_string(),
},
})
.await
.unwrap();
adapter
.update_assistant_draft(UpdateAssistantDraft {
message_ref: draft_ref.clone(),
reply: AssistantReply {
content: "partial answer".to_string(),
},
})
.await
.unwrap();

let finalized_ref = adapter
.finalize_assistant_message(FinalizeAssistantMessage {
reply: AssistantReply {
content: "partial answer complete".to_string(),
},
})
.await
.unwrap();

assert_eq!(
finalized_ref.as_str(),
draft_ref.as_str(),
"finalize must finalize the streamed draft in place, not create a new message"
);
let history = fixture
.thread_service
.list_thread_history(ThreadHistoryRequest {
scope: fixture.thread_scope.clone(),
thread_id: fixture.thread_id.clone(),
})
.await
.unwrap();
let assistant_rows: Vec<_> = history
.messages
.iter()
.filter(|message| message.kind == MessageKind::Assistant)
.collect();
assert_eq!(assistant_rows.len(), 1);
assert_eq!(assistant_rows[0].status, MessageStatus::Finalized);
assert_eq!(
assistant_rows[0].content.as_deref(),
Some("partial answer complete")
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether caller-level coverage already exists for append-error convergence
# and divergent duplicate finalize behavior.
rg -n -C3 'finalized_reply_for_run_matching|TranscriptWriteFailed|divergent|append_finalized_assistant_message' crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs

Repository: nearai/ironclaw

Length of output: 556


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the production finalize path and existing contract coverage around it.
rg -n -C4 'finalize_assistant_message|append_finalized_assistant_message|finalized_reply_for_run_matching|TranscriptWriteFailed|divergent' \
  crates/ironclaw_loop_support/src/lib.rs \
  crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs

# Map the relevant test file structure so we can read only the pertinent sections.
ast-grep outline crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs --view expanded

# Read the production finalize path around the reported lines.
sed -n '600,690p' crates/ironclaw_loop_support/src/lib.rs

# Read the nearby test region that already covers finalize behavior.
sed -n '2220,2395p' crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs

Repository: nearai/ironclaw

Length of output: 42985


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for any caller-level tests that exercise the append-failure fallback or divergence rejection.
rg -n -C3 'finalized_reply_for_run_matching|TranscriptWriteFailed|append_finalized_assistant_message|finalize_assistant_message\(' crates/ironclaw_loop_support/tests crates/ironclaw_loop_support/src

# Inspect the concrete error handling branch in the production code.
sed -n '620,675p' crates/ironclaw_loop_support/src/lib.rs

Repository: nearai/ironclaw

Length of output: 21931


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the production finalize path and nearby tests for the fallback branch.
sed -n '620,675p' crates/ironclaw_loop_support/src/lib.rs
sed -n '2250,2390p' crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs

# Find any other tests that mention the fallback/error-convergence path.
rg -n -C3 'finalized_reply_for_run_matching|TranscriptWriteFailed|append_finalized_assistant_message|divergent' crates/ironclaw_loop_support/tests

Repository: nearai/ironclaw

Length of output: 8517


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the production branch that handles append failure and divergence.
sed -n '620,675p' crates/ironclaw_loop_support/src/lib.rs

Repository: nearai/ironclaw

Length of output: 2790


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '2660,2805p' crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs

Repository: nearai/ironclaw

Length of output: 5173


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the candidate finalize tests that may already cover the fallback and divergence paths.
sed -n '2660,2805p' crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs

Repository: nearai/ironclaw

Length of output: 5173


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '2658,2798p' crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs

Repository: nearai/ironclaw

Length of output: 4969


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for a test that exercises divergent finalize content or the append-failure fallback.
rg -n -C4 'divergent|different content|TranscriptWriteFailed|append_finalized_assistant_message|finalized_reply_for_run_matching|mismatch' \
  crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs

# Read the concurrent-duplicate test continuation and the gated service implementation.
sed -n '2790,2865p' crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs
sed -n '4268,4298p' crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs

Repository: nearai/ironclaw

Length of output: 5899


Add a divergent-content finalize test. The concurrent-duplicate case already covers the append-failure convergence path; add a caller-level test that finalizing the same run with different content returns TranscriptWriteFailed and leaves history unchanged.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_loop_support/tests/thread_loop_support_contract.rs` around
lines 2255 - 2373, The finalize tests in
transcript_port_finalize_assistant_reply_is_idempotent_for_one_run and
transcript_port_finalize_finalizes_existing_streamed_draft_in_place only cover
duplicate-content convergence; add a new caller-level test that finalizing the
same run with different assistant content returns TranscriptWriteFailed. Reuse
ThreadFixture and ThreadBackedLoopTranscriptPort, invoke
finalize_assistant_message twice with mismatched content for the same
turn_run_id, and assert history stays unchanged with no extra assistant row.

Source: Path instructions

@railway-app

railway-app Bot commented Jun 30, 2026

Copy link
Copy Markdown

🚅 Deployed to the ironclaw-pr-5471 environment in ironclaw-ci-preview

Service Status Web Updated (UTC)
ironclaw ✅ Success (View Logs) Web Jun 30, 2026 at 9:56 pm

@serrrfirat serrrfirat closed this Jul 7, 2026

This branch was successfully deployed

No deployments
ironclaw-ci-preview / ironclaw-pr-5471 — 211783f5 Deployed Jun 30, 2026 by railway-app[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: low Changes to docs, tests, or low-risk modules size: M 50-199 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants