Repository navigation
fix(ai-vault): follow ZCode stored transcript order - #24584
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 10 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
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. Comment |
There was a problem hiding this comment.
ℹ️ No critical issues — the ordering logic faithfully mirrors upstream ZCode's reader. Two low-stakes notes below.
Reviewed changes
- New
zcodeTranscriptOrderhelper — builds a schema-awareORDER BYfragment: whenagent === 'zcode'and the table has asequencecolumn it orders bysequence IS NULL <dir>, sequence <dir>, time_created <dir>, <tiebreaker> <dir>, else it reproduces the pre-PR fragments exactly. - Capture, first-prompt, and preview readers — all three now order through the helper. Tie-breakers match upstream:
rowidfor sequenced messages,idfor sequenced parts. - Preview window — the zcode subquery now projects
m.rowid AS rowid, m.*so the outerDESCorder can referencesequence/rowid; the non-zcode path is untouched.DESCreverses upstream's ascending key, so NULL-sequence rows sort newest-first, mirroring how upstream places them last when oldest-first. - Tests — cover partially migrated schemas (
message,part,both), legacy NULL rows, and sequence-over-time ordering for first prompt, preview, and capture.
I verified the pinned upstream revision directly: migration 0014_message_part_sequence and 0015_message_part_sequence_backfill_and_guard define the intended NULL behavior, and the reader in repositories/messages.ts orders by sequence is null, sequence, time_created, rowid (messages) and sequence is null, sequence, time_created, id (parts). The helper reproduces these keys exactly, including the otherwise-surprising rowid-for-message / id-for-part split. OpenCode paths are unchanged. I also ran the ZCode test file (6 passed) and the related OpenCode AI Vault tests (36 passed).
ℹ️ Nitpicks
- The partially-migrated test only drives
kind: 'capture'(ASC), so the direction-dependent NULL branch in the preview (DESC) is untested. A mixed NULL/non-NULL session asserting the preview window would pin the one subtle branch the helper adds. - The helper comment "with legacy NULL rows last" holds only for
ASC; withDESCthesequence IS NULL ${direction}term puts NULL rows first. Since the helper is direction-parameterized, a clause noting that would prevent a future "fix" to the intentional reversal.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
b329456 to
07f777a
Compare

ELI5
ZCode saves the order of conversation turns separately from their timestamps. Orca now follows that saved order, so an imported or forked conversation shows its actual first prompt and latest turns.
What Changed
First-prompt loading, session previews, and full-text transcript capture use ZCode's message and part
sequencecolumns when present. Older databases and OpenCode keep their existing ordering. NULL sequence rows follow ZCode's legacy-row policy, and each table is checked independently.Why
ZCode CLI 0.16.9 at upstream revision
29628c9acdb81b703bbd4080c207a0e7ce5e276euses migration0014_message_part_sequenceand sequence-first readers. Sorting only by timestamps can select a later prompt as the first prompt and reverse text blocks. Extending Orca's existing SQLite reader keeps discovery, hidden-message filtering, and bounded previews together.Linked Issue
Follow-up to the native ZCode integration requested in #21756 and history implementation #23513. This does not resolve GLM account setup #23803 or quota #18506/#21757.
Visual Proof
Both screenshots use Orca's real session-details component in a hidden Electron renderer, with results from a database created by upstream ZCode's own storage API and synthetic conversation text. The baseline result was produced from the unchanged HEAD reader; the current first-prompt value was also verified through the real Electron IPC path. This is component proof, not an end-to-end history discovery test.
Before:
After:
Testing
ORCA_BACKGROUND_LAUNCH=1 pnpm tcandORCA_BACKGROUND_LAUNCH=1 pnpm run check:code-quality:changedpass. Targeted ZCode hooks, quota, readiness, and history checks: 85 tests passed; 3 pre-existing live tests skipped because their fixed-path fixtures are absent. Final shared-history rerun: 38 tests passed. The new regression failed before the fix. Coverage includes partially migrated schemas, legacy NULL sequence rows, hidden messages, OpenCode coexistence, and Windows resume-command construction.Built upstream CLI 0.16.9 from pinned source in a task-owned clone; verified
--version,--help, actual upstream SQLite store reads, and a real PTY resume launch in an isolated synthetic home. No authenticated quota request or paid model turn was performed. macOS was exercised; live Windows/Linux/SSH checks remain unverified.AI Disclosure
Implemented and verified with Codex.
Review
No new storage, status producers, wire fields, or credential handling. SQLite reads remain on the execution host and read-only. Folder workspace proof used a non-git directory. OpenCode ordering is retained.
Agent skill upstream boundary
Notes
Current upstream CLI credentials have also moved to
.zcode/v2/provider_config.jsonand the shared credential store; that separate quota/account migration remains follow-up work. The initial hidden app launch reconciled managed hooks before the isolated-profile safeguard was received; logs and scope are documented for the coordinator, and the owned app has been stopped.Checklist