Skip to content

feat(state): add parent_entry_id on the message table for fork support#2308

Open
hongqitai wants to merge 3 commits into
Hmbown:mainfrom
hongqitai:feat/fork-support-on-state
Open

feat(state): add parent_entry_id on the message table for fork support#2308
hongqitai wants to merge 3 commits into
Hmbown:mainfrom
hongqitai:feat/fork-support-on-state

Conversation

@hongqitai
Copy link
Copy Markdown

@hongqitai hongqitai commented May 28, 2026

Summary

Refs #2082 add fork support to state crate, add parent_entry_id to message table and current_leaf_id to threads table

Change

  • change some structs and other crate to add fields to match the database
  • init_schema: use user_version to determine whether an update is needed, on update do old migration first then add new column and use created_at to make chain
  • append_message: add message under current leaf message and set current to new
  • list_messages: list the parent chain of current leaf
  • (Add) list_leaf_messages: return messages on leaf node of the message tree
  • (Add) fork_at_message: add message under specified message and set current to new
  • (Test) init_schema_migration: make sure it can migration from existing sessions
  • (Test) test_fork: test forks a 5-message session and proves both leaves are queryable independently

Testing

  • [ x ] cargo fmt --all -- --check
  • [ x ] cargo clippy --workspace --all-targets --all-features
  • [ x ] cargo test --workspace --all-features

Checklist

  • [ x ] Updated docs or comments as needed
  • [ x ] Added or updated tests where relevant
  • [ x ] Verified TUI behavior manually if UI changes

Greptile Summary

This PR adds fork support to the state crate by introducing parent_entry_id on the messages table and current_leaf_id on the threads table, enabling a message-tree structure where conversations can branch from any historical message.

  • init_schema is now versioned via PRAGMA user_version: on first open it runs CREATE TABLE IF NOT EXISTS for all tables and immediately ALTER TABLE to add the two new columns, sets user_version = 1, and wraps everything in a BEGIN/COMMIT to make the migration atomic.
  • append_message is rewritten as a transaction that reads current_leaf_id, inserts the new message with that value as parent_entry_id, then advances current_leaf_id; list_messages switches from a flat ORDER BY created_at to a recursive CTE that walks the parent chain from current_leaf_id.
  • New public API: fork_at_message inserts a child under an arbitrary existing message and sets it as the new current leaf; list_leaf_messages returns all tip nodes; set_current_leaf_id lets callers switch the active branch.

Confidence Score: 3/5

The migration parent-chain reconstruction uses strict less-than on a seconds-precision timestamp, which silently truncates conversation history for any existing thread where two or more messages share the same second.

Any thread with rapid message exchanges — the common case for LLM tool-call/response cycles — will have messages with identical created_at values. The migration sets parent_entry_id to NULL for all of them, breaking the ancestor chain. list_messages then returns only the fragment from current_leaf_id to the first orphan, dropping older messages permanently and irreversibly.

crates/state/src/lib.rs — the UPDATE messages SET parent_entry_id subquery in the migration block (lines 243–250).

Important Files Changed

Filename Overview
crates/state/src/lib.rs Core state layer rewritten to support fork: adds parent_entry_id/current_leaf_id, new CTE-based list_messages, fork_at_message, list_leaf_messages, set_current_leaf_id. Migration timestamp collision (strict < on created_at) can break the parent chain for existing sessions with same-second messages.
crates/core/src/lib.rs Adds current_leaf_id: None to the ThreadMetadata literal in persist_thread; upsert_thread's ON CONFLICT clause intentionally omits current_leaf_id so the stored value is preserved.
crates/state/tests/parity_state.rs Adds two new tests: init_schema_migration (migration from a pre-existing schema) and test_fork (full fork/leaf/clear lifecycle). A stray dbg! macro is left in test_fork.

Comments Outside Diff (1)

  1. crates/core/src/lib.rs, line 624-647 (link)

    P1 persist_thread hard-codes current_leaf_id: None, wiping it on every resume

    persist_thread builds a ThreadMetadata literal with current_leaf_id: None. Because upsert_thread's ON CONFLICT clause includes current_leaf_id = excluded.current_leaf_id, any call to persist_thread on an existing thread writes NULL over the value that append_message painstakingly maintained. resume_thread_with_history calls persist_thread before it conditionally appends history (line 502), so resuming a thread that has no new history clears current_leaf_id permanently — after which list_messages returns an empty list because the CTE base case WHERE id = NULL never matches.

    Fix in Codex Fix in Claude Code Fix in Cursor

Fix All in Codex Fix All in Claude Code Fix All in Cursor

Reviews (5): Last reviewed commit: "fix: upsert_thread will not set current_..." | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

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

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 introduces message branching and forking capabilities to the state store by adding current_leaf_id to threads and parent_entry_id to messages, alongside database schema migrations and recursive CTE queries. The code reviewer identified a critical bug where the recursive CTE query returns messages in reverse chronological order, and suggested handling limit == 0 early. Additionally, the reviewer pointed out that using created_at in migrations can cause issues due to duplicate timestamps and recommended using the auto-incrementing id instead. Other feedback includes simplifying an Option<String> type to String and updating test assertions to match chronological ordering.

Comment thread crates/state/src/lib.rs
Comment thread crates/state/src/lib.rs
Comment thread crates/state/src/lib.rs Outdated
Comment thread crates/state/tests/parity_state.rs
Comment thread crates/state/tests/parity_state.rs
Comment thread crates/state/tests/parity_state.rs Outdated
Comment thread crates/state/tests/parity_state.rs
Comment thread crates/state/src/lib.rs
Comment thread crates/state/src/lib.rs Outdated
Comment thread crates/state/src/lib.rs
Ok(out)
}

pub fn set_current_leaf_id(&self, thread_id: &str, current_leaf_id: &str) -> Result<()> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 set_current_leaf_id accepts current_leaf_id as &str even though the underlying column is INTEGER and every other call-site works with i64. SQLite silently coerces the string, but callers are forced to call .to_string() on an integer they already hold, which is type-unsafe. The parameter should be i64.

Suggested change
pub fn set_current_leaf_id(&self, thread_id: &str, current_leaf_id: &str) -> Result<()> {
pub fn set_current_leaf_id(&self, thread_id: &str, current_leaf_id: i64) -> Result<()> {

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Codex Fix in Claude Code Fix in Cursor

Comment thread crates/state/src/lib.rs
@hongqitai
Copy link
Copy Markdown
Author

hongqitai commented May 28, 2026

fix all problem without coding style, because of I find other code in this crate use "&str" to take int value

@hongqitai hongqitai force-pushed the feat/fork-support-on-state branch 2 times, most recently from 83b848f to 8cb6b91 Compare May 28, 2026 10:38
Comment thread crates/state/src/lib.rs
…ill also clean current_leaf_id,

update tests
@hongqitai hongqitai force-pushed the feat/fork-support-on-state branch from 8cb6b91 to 734127f Compare May 28, 2026 10:53
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.

1 participant