Skip to content

FROM bug/697-can-checkpoint-off-thread TO development#698

Open
ryaneggz wants to merge 2 commits into
developmentfrom
bug/697-can-checkpoint-off-thread
Open

FROM bug/697-can-checkpoint-off-thread TO development#698
ryaneggz wants to merge 2 commits into
developmentfrom
bug/697-can-checkpoint-off-thread

Conversation

@ryaneggz

@ryaneggz ryaneggz commented Jan 25, 2026

Copy link
Copy Markdown
Collaborator

Closes #697

Summary by CodeRabbit

  • New Features

    • Added branching checkpoint support so edited messages can be saved as off-thread checkpoints (enables message-specific branching).
  • Bug Fixes

    • Recorded fix for checkpointing behavior (bug/697).
  • Documentation

    • Added a brief specification for the checkpoint branching feature and updated the changelog; removed an internal planning task document.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Jan 25, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Adds a changelog entry, introduces a new SPEC for checkpoint branching, removes a planning task doc, and updates frontend chat message and chat hook APIs to pass checkpoint metadata (metadataOverrides) through submission and SSE handling to enable branching off checkpoints.

Changes

Cohort / File(s) Summary
Changelog
Changelog.md
Added a Fixed entry bug/697-can-checkpoint-off-thread (2026-01-24) under v0.0.2-rc141.
New spec
.claude/specs/feat-697-checkpoint-branching/SPEC.md
Added new specification file "Feature 697 allow checkpoint" (minimal content).
Removed task doc
RALPH_TASK.md
Deleted the Human-in-the-Loop API Routes planning/task document and all its contents.
UI message component
frontend/src/components/lists/ChatMessages.tsx
Expanded Message component handleSubmit prop signature to (content, images?, metadataOverrides?) and updated local save flow to call it with checkpoint_id: message.id.
Chat hook / SSE flow
frontend/src/hooks/useChat.ts
Extended handleSubmit, handleSSEUnified, and handleSSE signatures to accept metadataOverrides; propagate and merge metadataOverrides into enriched metadata and adjust values-stream handling to align latest user message and file associations.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant MessageComponent as Message
    participant UseChatHook as useChat.handleSubmit
    participant SSE as Server(SSE)
    participant History as History/Store

    User->>Message: edit & save (content)
    Message->>UseChatHook: handleSubmit(content, images, { checkpoint_id })
    UseChatHook->>SSE: open SSE / send request (includes merged metadata)
    SSE-->>UseChatHook: incremental events (values/streams)
    UseChatHook->>History: merge messages/files, associate with checkpoint_id
    UseChatHook-->>Message: update UI with new history/messages
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I stitched a branch where checkpoints bloom,
Off-thread hops now find room,
A spec and changelog, a doc set free,
Messages carry a tiny key,
Hop forward—branching's bright as can be!

🚥 Pre-merge checks | ✅ 2 | ❌ 3
❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes deletion of RALPH_TASK.md (unrelated Human-in-the-Loop documentation) and a new spec file, which appear to be outside the checkpoint branching feature scope. Remove the deletion of RALPH_TASK.md and the addition of .claude/specs/feat-697-checkpoint-branching/SPEC.md, or clarify their relationship to issue #697.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title references the source branch (bug/697-can-checkpoint-off-thread) but does not clearly summarize the main change in developer-friendly terms. Revise the title to be more descriptive, e.g., 'Allow users to branch off checkpoints' or 'Enable checkpoint branching feature'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR implements checkpoint branching by expanding handleSubmit signatures to accept metadataOverrides with checkpoint_id, enabling message-specific branching as required by issue #697.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bug/697-can-checkpoint-off-thread

Tip

🧪 Unit Test Generation v2 is now available!

We have significantly improved our unit test generation capabilities.

To enable: Add this to your .coderabbit.yaml configuration:

reviews:
  finishing_touches:
    unit_tests:
      enabled: true

Try it out by using the @coderabbitai generate unit tests command on your code files or under ✨ Finishing Touches on the walkthrough!

Have feedback? Share your thoughts on our Discord thread!


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 and usage tips.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@Changelog.md`:
- Line 37: Move the changelog entry "bug/697-can-checkpoint-off-thread
(2026-01-24)" out of the v0.0.2-rc141 section and insert it into the
v0.0.2-rc142 section (near other 2026-01-24 entries such as
"feat/694-show-subagent-tool-calls") so the date grouping matches; update
ordering if necessary to keep chronological order within v0.0.2-rc142 and remove
the original line from its current location.
🧹 Nitpick comments (1)
Changelog.md (1)

37-37: Consider improving the changelog entry clarity.

While the entry follows the existing pattern of listing branch names, "can-checkpoint-off-thread" doesn't clearly communicate what was fixed. Based on the PR objective, consider adding a brief description like "Enable branching from checkpoints in off-thread context" to help users understand the change without referencing the issue.

Comment thread Changelog.md
- FEAT: Truncate Tool Calls. Allow extension (cli) #54

### Fixed
- bug/697-can-checkpoint-off-thread (2026-01-24)

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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check git tags and history for bug/697

echo "=== Git tags (most recent first) ==="
git tag --sort=-creatordate | head -20

echo -e "\n=== Commit history for bug/697-related changes ==="
git log --oneline --all --grep="697" --grep="checkpoint" --grep="off-thread" -i --since="2026-01-20" | head -20

echo -e "\n=== Check git log for releases/tags ==="
git log --oneline --all --grep="release\|rc141\|rc142" -i --since="2026-01-20" | head -20

Repository: ruska-ai/orchestra

Length of output: 191


🏁 Script executed:

cat -n Changelog.md | head -50

Repository: ruska-ai/orchestra

Length of output: 2138


Move this entry to v0.0.2-rc142.

The bug/697 fix is dated 2026-01-24, which matches the most recent entries in v0.0.2-rc142 (e.g., line 11: feat/694-show-subagent-tool-calls (2026-01-24)). It should be listed under that version instead of v0.0.2-rc141, whose Changed entries range from 2026-01-08 to 2026-01-10.

🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

37-37: Unordered list indentation
Expected: 0; Actual: 2

(MD007, ul-indent)

🤖 Prompt for AI Agents
In `@Changelog.md` at line 37, Move the changelog entry
"bug/697-can-checkpoint-off-thread (2026-01-24)" out of the v0.0.2-rc141 section
and insert it into the v0.0.2-rc142 section (near other 2026-01-24 entries such
as "feat/694-show-subagent-tool-calls") so the date grouping matches; update
ordering if necessary to keep chronological order within v0.0.2-rc142 and remove
the original line from its current location.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@frontend/src/components/lists/ChatMessages.tsx`:
- Around line 70-78: Remove the debug console.log from the handleSave function
and correct its indentation to match the file's 2-space Prettier style (align
with handleTextareaChange). Specifically, edit the handleSave handler (the async
function that calls handleSubmit with editedContent and { checkpoint_id:
message.id } and then calls setIsEditingText(false) and setIsEditing(false)) to
delete the console.log line and reformat the block with 2-space indentation so
it is consistent with neighboring functions.
🧹 Nitpick comments (1)
frontend/src/hooks/useChat.ts (1)

490-503: Inconsistent indentation and direct array mutation.

Two concerns with this block:

  1. Indentation: Lines 490-503 appear to have inconsistent indentation compared to the surrounding code blocks (e.g., the if (streamMode === "error") block at line 462). This violates the 2-space Prettier indentation guideline.

  2. Direct mutation: Line 502 directly mutates the history array parameter (history[lastUserIndex] = latestUserFromValues). While this appears intentional for aligning server state with local state, mutating function parameters can lead to subtle bugs. Consider documenting this behavior or creating a new array if immutability is preferred.

🔧 Fix indentation and add clarifying comment
-	if (streamMode === "values") {
-		const valuesData = payload[1];
-
-		// Get latest user message from valuesData.messages and history
-		const latestUserFromValues = valuesData.messages
-			?.slice()
-			.reverse()
-			.find((msg: any) => ["user", "human"].includes(msg.type ?? msg.role));
-
-		const lastUserIndex = history.findLastIndex((item) => item.role === "user");
-
-		if (latestUserFromValues && lastUserIndex !== -1) {
-			history[lastUserIndex] = latestUserFromValues;
-		}
+    if (streamMode === "values") {
+      const valuesData = payload[1];
+
+      // Align optimistic user message with server-confirmed message (mutates history intentionally)
+      const latestUserFromValues = valuesData.messages
+        ?.slice()
+        .reverse()
+        .find((msg: any) => ["user", "human"].includes(msg.type ?? msg.role));
+
+      const lastUserIndex = history.findLastIndex((item) => item.role === "user");
+
+      if (latestUserFromValues && lastUserIndex !== -1) {
+        history[lastUserIndex] = latestUserFromValues;
+      }

As per coding guidelines: "React code uses Prettier's 2-space indentation"

Comment on lines +70 to +78
// Handle save
const handleSave = async (e: React.MouseEvent) => {
e.stopPropagation();
// Pass checkpoint_id directly to handleSubmit for branching from this message
console.log("Saving edited message with checkpoint_id:", message.id);
await handleSubmit(editedContent, [], { checkpoint_id: message.id });
setIsEditingText(false);
setIsEditing(false);
};

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.

⚠️ Potential issue | 🟡 Minor

Remove debug console.log and fix indentation.

  1. Debug code: Line 74 contains a console.log statement that should be removed before merging.

  2. Indentation: This block has inconsistent indentation compared to adjacent functions like handleTextareaChange (lines 46-53).

🧹 Remove console.log and fix indentation
-	// Handle save
-	const handleSave = async (e: React.MouseEvent) => {
-		e.stopPropagation();
-		// Pass checkpoint_id directly to handleSubmit for branching from this message
-		console.log("Saving edited message with checkpoint_id:", message.id);
-		await handleSubmit(editedContent, [], { checkpoint_id: message.id });
-		setIsEditingText(false);
-		setIsEditing(false);
-	};
+    // Handle save
+    const handleSave = async (e: React.MouseEvent) => {
+      e.stopPropagation();
+      // Pass checkpoint_id directly to handleSubmit for branching from this message
+      await handleSubmit(editedContent, [], { checkpoint_id: message.id });
+      setIsEditingText(false);
+      setIsEditing(false);
+    };

As per coding guidelines: "React code uses Prettier's 2-space indentation"

📝 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
// Handle save
const handleSave = async (e: React.MouseEvent) => {
e.stopPropagation();
// Pass checkpoint_id directly to handleSubmit for branching from this message
console.log("Saving edited message with checkpoint_id:", message.id);
await handleSubmit(editedContent, [], { checkpoint_id: message.id });
setIsEditingText(false);
setIsEditing(false);
};
// Handle save
const handleSave = async (e: React.MouseEvent) => {
e.stopPropagation();
// Pass checkpoint_id directly to handleSubmit for branching from this message
await handleSubmit(editedContent, [], { checkpoint_id: message.id });
setIsEditingText(false);
setIsEditing(false);
};
🤖 Prompt for AI Agents
In `@frontend/src/components/lists/ChatMessages.tsx` around lines 70 - 78, Remove
the debug console.log from the handleSave function and correct its indentation
to match the file's 2-space Prettier style (align with handleTextareaChange).
Specifically, edit the handleSave handler (the async function that calls
handleSubmit with editedContent and { checkpoint_id: message.id } and then calls
setIsEditingText(false) and setIsEditing(false)) to delete the console.log line
and reformat the block with 2-space indentation so it is consistent with
neighboring functions.

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.

bug: Users should have ability to branch off checkpoint

1 participant