Repository navigation
Preserve OpenCode reasoning and recorded patches in native history - #24790
Conversation
OpenCode database and synthetic row paths are not text transcripts. Use the vault preview or captured pane context, preserving actual transcript paths containing a hash and supporting both OpenCode lanes and Windows paths. Adapted the intent of #11859 and extended it to actual installed v2 vault rows. Co-authored-by: mrcha033 <mrcha033@users.noreply.github.com>
Reuse the bounded AI Vault SQLite worker for v1 and v2 session pages and live updates. Keep terminal input as the real execution path and pace OpenCode Stop through its two-Escape interrupt. Co-authored-by: xodmd45-ctrl <xodmd45-ctrl@users.noreply.github.com>
Separate genuine reasoning from answer blocks in both native SQLite schemas and retain recorded patches as completed patch tools. Keep each database row together at page boundaries so the existing raw-row cursors cannot drop half of a mixed row. Adapted from @akhan157's OpenCode native history work in #13287 at bb661d1; retains the current bounded reader and account discovery instead of restoring the older capture and cursor implementation. Verified against genuine private installed 2.0.16 and official 1.18.30 CLI ingestion. Co-authored-by: Adnan Khan <adnank11427@gmail.com>
Preserve the native reader's bounded OpenCode row groups in paired reads and snapshot/replacement frames so a second presentation-count slice cannot drop reasoning while advancing the database cursor. Sort derived reasoning before its answer under the same provider timestamp while retaining journal order. These two boundaries were reproduced with genuine installed 2.0.16 and official 1.18.30 sessions in a hidden desktop renderer and the current mobile view over an actual authenticated encrypted pairing. Completes the semantic presentation from @akhan157's #13287 without importing its older clipping or cursor implementation. Co-authored-by: Adnan Khan <adnank11427@gmail.com>
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one minor client-side finding inline.
Reviewed changes
- Reasoning split in the reader —
opencodeMessagesnow returns areasoningmessage (<id>:reasoning) before the answer, drops the reasoning text out of the answer's blocks, and mapspatchparts to completedtool-callblocks carrying{ hash, files }. - Pair-preserving pagination — the v1 reader extends a page's retained slice so a raw-row cursor never lands inside a reasoning/answer pair; v2 pushes both messages per row atomically; a pair is counted as one cursor unit.
- Shared transcript ordering —
compareNativeChatMessagesByTimesorts a:reasoningmessage immediately before its answer using the base id, keeping provider timestamps authoritative. - RPC window exemption —
windowForClientskips count-windowing for OpenCode (opencodeandopencode2both resolve toopencode) so pairs reach desktop and mobile intact. - Tests — semantic-fidelity (v1/v2 paging, streaming replace/append), read-bounds, comparator-order, and paired RPC-page suites.
ℹ️ Nitpicks
- No test covers the reasoning-only → reasoning+answer transition, where a row's reasoning id changes from
row.idtorow.id:reasoning. The existing streaming test mutates the answer of an already-mixed()row, so a regression in the id-swap path would not fail it.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
Compose the complete original fidelity component with the reviewed live-window pair correction on frozen current main. Retain native row placeholders, normalized limits, and aggregate byte guards. Original report and reasoning fix: @adnanmujkanovic (#13287). Original-Public-Head: b5937bc Reviewed-Focused-Fix: b9a92b6 Component-Common-Core: 581e9ae Frozen-Main: a2896f5
|
The complete 13-file fidelity change now preserves the native reader's oversized-row and page limits. Reasoning/answer pairs also remain together at a live mobile window cutoff. These retained hidden Electron screenshots use genuine installed OpenCode 2.0.16 records and the production mobile session hook/components, with controlled frame delivery. They compare the original 40-row cutoff with the corrected 41-row window. They are from the earlier focused pair-window build, not a fresh current-main renderer run or physical mobile device.
Fresh current-main validation ran 15 actual installed CLI turns in an empty private profile. The production read/subscription/RPC/sanitization/mobile merger pipeline retained all 41 expected messages and unique IDs. The actual stream emitted a replacement frame; the old raw-count merger also passes that replacement path. A separately labeled controlled appended-delivery replay of the same genuine records reproduces old40/new41. No live pairing/relay/device proof is claimed for this fresh run. |
There was a problem hiding this comment.
ℹ️ No new critical issues — one minor consistency suggestion inline.
Reviewed changes
- Oversized-row handling in both readers — the SQL now measures row bytes with
length(CAST(data AS BLOB))againstOPENCODE_TRANSCRIPT_MAX_ROW_BYTES, and an oversized message/part row materializes ahistory-item-too-largetext block instead of throwing the page away. - Pair-aware live-window bound —
boundNativeChatWindowkeeps the precedingreasoningwhen the cutoff would split it from a matching answer (at mostlimit + 1), via the extractednativeChatSemanticRowIdalso used by the comparator. - Tests — added oversized-row paging (v1/v2),
native-chat-merge-pairs, andmobile-native-chat-pair-windowcoverage; updated the streaming semantic-fidelity case.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughOpenCode transcript parsing emits separate reasoning, content, and omission messages. SQLite readers attach transcript offsets and keep messages from the same row together during pagination. Shared ordering and windowing use semantic row identity to preserve complete pairs. RPC pages and append batches use byte bounds for OpenCode. Mobile rendering displays marked history-size notices as selectable, scaled text. Native runtime rebuild timeouts depend on the module and platform. Priority: ➖ Normal Merge Risk: 🔵 Low · up to Users with a malformed OpenCode TUI config may not get the status plugin installed or refreshed because the new TUI registration error aborts the install step. The fix is localized; otherwise the change looks mergeable. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The inspected changes add checks around session identity and repository writes. No newly exploitable security issue was established, but session cleanup and recovery behavior and some remote compatibility paths remain incompletely verified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❓ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 14.10% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 78 functions across 76 files. (72 skipped: 2 unsupported, 70 over the file limit.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
4995651d-81d9-4ebb-92cb-903a580d794f
📒 Files selected for processing (13)
mobile/src/session/mobile-native-chat-pair-window.test.tssrc/main/native-chat/transcript-opencode-part-blocks.tssrc/main/native-chat/transcript-opencode-read-bounds.test.tssrc/main/native-chat/transcript-opencode-semantic-fidelity.test.tssrc/main/native-chat/transcript-opencode-sqlite-query.tssrc/main/native-chat/transcript-opencode2-sqlite.tssrc/main/runtime/rpc/methods/native-chat-opencode-semantic-pages.test.tssrc/main/runtime/rpc/methods/native-chat.tssrc/shared/native-chat-merge-pairs.test.tssrc/shared/native-chat-merge.tssrc/shared/native-chat-reasoning-order.test.tssrc/shared/native-chat-transcript-projection.tssrc/shared/native-chat-types.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
|
The exact 7323d83 source is now based on main and merges cleanly. Temporarily closing and reopening to trigger the normal PR and mobile checks: the earlier synchronize event had conflicts against the old native stack, and the base edit does not trigger these workflows. The branch and reviewed source are retained. |
|
Fresh installed OpenCode 2.0.16 wrote the actual conversation records under private profiles. These unedited 430×900 CDP screenshots render production mobile components in a hidden Orca app on exact f63bf8e; published 2a4fc79 retains all20 qualified postimages unchanged. The before notice is an exact732 producer/consumer module control over the same genuine database in the current hidden app. This is a controlled local model and renderer proof; it does not claim a physical phone, relay/WebSocket or SSH run.
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes (since the prior pullfrog review at 7323d830f5):
- Omitted rows render as localized system notices — the oversize placeholder now travels as its own
role: 'system'message carrying ahistory-item-too-largepresentationkey (opencodeMessages+agentSessionHostStatusBody), and v2's sealed-row branch forces the system role. Mobile renders any host-status presentation block as selectable muted text; desktop routes it toNativeChatNoticeRow. This addresses the prior inline finding (transcript-opencode-part-blocks.ts:51). - OpenCode RPC pages are byte-bounded, not just count-bounded — new
native-chat-rpc-page-bounds.tskeeps whole row groups newest-first underHISTORY_PAGE_CONTENT_BUDGET_BYTES, substitutes one system notice for an oversized newest group, and re-pointsbeforeOffset; live appends are split into per-frame batches on the same budget, withlifecycleattached to the last batch only. - Row grouping is now cursor-aware —
transcriptOffsetis added to every OpenCode message,nativeChatMessagesShareTranscriptRowgroups by it (falling back to the:reasoningid convention for older projections), andboundNativeChatWindowwalks back over whole rows so a live cutoff never splits reasoning/answer/omission. - Reader pagination keeps split rows intact — v1 extends the retained slice while a cut lands mid-row; v2 emits each row's messages atomically; both stamp
transcriptOffsetand reusenativeChatSemanticRowIdin the comparator. - Tests — new byte-budget, semantic-page, pair-window, merge-pair, reasoning-order and mobile-notice coverage exercises the real SQLite readers, RPC serializer and E2EE admission with exact-value assertions.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes (since the prior pullfrog review at 2a4fc79):
- Worker oversize-notice expectations corrected — the parametrized
v1-message/v1-part/v2-messagecase insession-scanner-opencode-native-worker.test.tsnow asserts the omitted-row message isrole: 'system'and carriespresentation: 'history-item-too-large', matching the producer that landed in2a4fc79. Test-only change; the suite passes (6/6) on the checked-out head.
ℹ️ Unrelated red check
The mobile web app bundle job is red at this head (config/scripts/mobile-web-app-html-preview-render.test.mjs:619, a missing frame-src CSP report) while the parent commit 2a4fc79 passed it. This PR neither touches that test nor any CSP source, and this commit only edits a worker test, so the failure does not appear to be caused by the delta — it reads as a browser-test flake.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
Preserve all21 accepted source postimages, original13 plus reviewed extensions and worker fixture. No owned-path overlap exists with current main; all unowned main entries remain exact.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
661e143e-7373-4893-9b92-a1bdb9d421b7
📒 Files selected for processing (126)
.github/workflows/pr.ymlconfig/build-plugins/jsonc-parser-esm.tsconfig/scripts/build-relay.mjsconfig/scripts/git-binary-compatibility-workflow.test.mjsconfig/scripts/mobile-web-app-html-preview-render.test.mjsconfig/scripts/mobile-web-app-preview-arm-driver.mjsconfig/scripts/pr-code-change-scope.mjsconfig/scripts/runtime-serve-terminal-smoke.mjsdocs/reference/jcode-hook-events.mdsrc/cli/handlers/search.test.tssrc/main/agent-hooks/server-opencode-tui-identity.test.tssrc/main/agent-hooks/server/server-ingest-normalization.tssrc/main/agent-hooks/server/server-runtime-env.tssrc/main/ai-vault-search/session-search-agent-compatibility.test.tssrc/main/ai-vault-search/session-search-scope-entry-points.test.tssrc/main/ai-vault-search/session-search-scope-service.test.tssrc/main/ai-vault-search/session-search-service-registry.test.tssrc/main/ai-vault-search/session-search-service-registry.tssrc/main/git/command-runner/git-exec-file.tssrc/main/git/command-runner/git-exec-options.tssrc/main/git/local-repo-ref-maintenance-probe-retry.test.tssrc/main/git/local-repo-ref-maintenance.test.tssrc/main/git/local-repo-ref-maintenance.tssrc/main/git/remove-worktree-branch-cleanup.test.tssrc/main/git/repo-maintenance-admission.test.tssrc/main/git/repo-pack-index-maintenance.test.tssrc/main/git/repo-pack-index-maintenance.tssrc/main/git/runner-network-ssh-policy.test.tssrc/main/git/runner-wsl-network-ssh-policy.test.tssrc/main/git/source-control/branch-compare.tssrc/main/git/status-branch-compare-real-ref.test.tssrc/main/git/status-branch-compare.test.tssrc/main/git/worktree-branch-removal-host.test.tssrc/main/git/worktree-branch-removal.tssrc/main/git/worktree-preparation-discard-real-git.test.tssrc/main/git/worktree-preparation-discard.tssrc/main/git/worktree-preparation-lock-real-git.test.tssrc/main/git/worktree-preparation-lock.test.tssrc/main/git/worktree-preparation-lock.tssrc/main/git/worktree-rebase-update-refs-real-git.test.tssrc/main/git/worktree-sparse-checkout-cache.tssrc/main/ipc/ai-vault-search.test.tssrc/main/ipc/filesystem/filesystem-git-pull-request-generation-handlers.tssrc/main/jcode/hook-gate-script.test.tssrc/main/jcode/hook-service.tssrc/main/opencode/hook-plugin-legacy-tui-ownership.test.tssrc/main/opencode/hook-service-legacy-acl.test.tssrc/main/opencode/hook-service.test.tssrc/main/opencode/opencode-legacy-plugin-loader-fixture.tssrc/main/opencode/opencode-legacy-tui-fixture.tssrc/main/opencode/status-plugin-delivery-source.tssrc/main/opencode/status-plugin-endpoint-source.tssrc/main/opencode/status-plugin-legacy-tui-source.tssrc/main/opencode/status-plugin-post-source.tssrc/main/opencode/status-plugin-runtime-state-source.tssrc/main/opencode/status-plugin-shared-server-capability.test.tssrc/main/opencode2/status-plugin-setup-source.tssrc/main/opencode2/status-plugin-tui-source.tssrc/main/providers/ssh-git-provider.tssrc/main/providers/ssh-review-draft-context.test.tssrc/main/providers/ssh-review-draft-context.tssrc/main/runtime/rpc/methods/ai-vault-search.test.tssrc/main/runtime/runtime-git-generation-context.tssrc/main/ssh/relay-native-dependency-coverage.test.tssrc/main/text-generation/pull-request-context.tssrc/main/zsh-scoped-histfile.live-shell.test.tssrc/main/zsh-startup-hook-pty-harness.tssrc/preload/api/ai-vault-bridge.tssrc/relay/agent-hook-endpoint-coordinates.tssrc/relay/agent-hook-opencode-tui-identity.test.tssrc/relay/agent-hook-request.tssrc/relay/agent-hook-server-contract.tssrc/relay/agent-hook-server.tssrc/relay/agent-hook-spool-ingest.tssrc/relay/fs-handler-ripgrep-fallback.test.tssrc/relay/fs-list-files-fallback-chain.tssrc/relay/git-handler-branch-cleanup.test.tssrc/relay/git-handler-branch-cleanup.tssrc/relay/git-handler-branch-compare.test.tssrc/relay/git-handler-branch-diff.test.tssrc/relay/git-handler-diff-read-coalescing.test.tssrc/relay/git-handler-object-diff-operations.tssrc/relay/git-handler-ops.tssrc/relay/git-handler-registration.tssrc/relay/git-handler-review-draft.test.tssrc/relay/git-handler-utils.tssrc/relay/git-review-draft-binary-compatibility.test.tssrc/relay/pty-handler-agent-launch-token.test.tssrc/relay/pty-handler.tssrc/relay/relay-agent-hook-runtime.tssrc/relay/relay-endpoint-credential-publication.test.tssrc/relay/session-search-transport.test.tssrc/relay/skill-upload-multi-relay.integration.test.tssrc/relay/subprocess.test.tssrc/renderer/src/web/preload-api/web-ai-vault-search-compat.test.tssrc/shared/agent-hook-listener-opencode-tui-identity.test.tssrc/shared/agent-hook-listener-relay-dependency.test.tssrc/shared/agent-hook-listener.tssrc/shared/agent-hook-listener/endpoint-publication.tssrc/shared/agent-hook-listener/opencode-session-registry.tssrc/shared/agent-hook-spool.tssrc/shared/ai-vault-search-agent-compatibility.tssrc/shared/ai-vault-search-agent-history.test.tssrc/shared/ai-vault-search-client.tssrc/shared/ai-vault-search-contract.test.tssrc/shared/ai-vault-search-contract.tssrc/shared/ai-vault-search-qoder-client.test.tssrc/shared/child-process/__fixtures__/child-process-import-allowlist.txtsrc/shared/child-process/__fixtures__/windows-console-visibility-allowlist.txtsrc/shared/child-process/windows-console-visibility.test.tssrc/shared/git-binary-compatibility.test.tssrc/shared/git-change-list.test.tssrc/shared/git-change-list.tssrc/shared/git-resolution-binary-compatibility.test-cases.tssrc/shared/git-ssh-policy-env.test.tssrc/shared/git-ssh-policy-env.tssrc/shared/git-worktree-admin.test.tssrc/shared/git-worktree-admin.tssrc/shared/opencode-tui-config-registration.test.tssrc/shared/opencode-tui-config-registration.tssrc/shared/opencode-tui-plugin-install.tssrc/shared/repo-maintenance-schedule.tssrc/shared/repo-ref-maintenance-policy.tssrc/shared/repo-ref-maintenance.tssrc/shared/review-draft-context-error.tstests/e2e/cross-version-wire/qoder-history-search-downgrade.unit.test.ts
💤 Files with no reviewable changes (4)
- src/shared/child-process/fixtures/child-process-import-allowlist.txt
- src/shared/child-process/fixtures/windows-console-visibility-allowlist.txt
- src/shared/ai-vault-search-qoder-client.test.ts
- src/relay/git-handler-utils.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
Preserve every accepted component postimage and all unowned main entries. Retain the public component ancestry and prior source-qualified proof.
ELI5
OpenCode history keeps reasoning with its answer and shows recorded patches. Large histories fit the mobile connection, and a reply too large to display gets a quiet notice instead of looking like an answer.
What Changed
All 13 accepted fidelity paths remain included. Both SQLite readers keep their existing row/page limits and project an omission as a system notice with the existing presentation key. Desktop uses its existing translations; mobile uses the existing shared English copy and muted text token.
The RPC path budgets sanitized JSON while retaining complete database-row groups, and adjusts pagination when older groups do not fit. A new optional row cursor keeps reasoning, answer and omission together; old peers retain the existing ID fallback. Existing frame types and lifecycle handling remain in use. The strict template-literal warning is fixed by capturing the checked update time before mapping.
Why
The reader's 16 MiB guard does not ensure a response fits the mobile connection's 4 MiB limit. Applying the existing history-page byte budget after sanitization bounds the actual payload without splitting recorded content or adding another transport. Existing notice presentation and mobile styles supply the display behavior.
Linked Issue
Refs #13287, with credit to Adnan Khan (@akhan157). This PR preserves its complete 21-file fidelity fix and the ancestry of the accepted work. The broader original contribution in #13287, mobile-display #14185 and Korean-input #19156 remain open.
Visual Proof
Fresh 430×900 hidden-renderer proof uses genuine installed OpenCode2.0.16 records and production mobile components: the complete final41-row pair and matching oversized-reply notice before/after. The notice before image uses the original producer/UI module over the same genuine database inside the current hidden app; it is not a separate old app or physical phone. Fresh inspected screenshots.
Complete final41-row pair.
Earlier fidelity screenshots remain historical evidence:
Earlier pair-window before/after separately compares the original40-row cutoff with the corrected41-row window.
Testing
CI exposed three stale oversized-row expectations in the production shared-worker test. The exact published-source control reproduced all three failures. The corrected test checks the system notice role and presentation metadata; all six worker controls pass, retaining row identity, repeat-read equality, and older-page checks. Only that fixture changed after the 20-path runtime qualification; formatting, lint, and all six changed-line gates pass.
Fresh real installed OpenCode2.0.16:15 successful standalone turns and a separate >2 MiB reply in empty private HOME/all4XDG/temp/userData, using a deterministic loopback model. The CLI wrote its own database records.
Exact qualified f63:75 mobile tests, four production typechecks, build, test-typecheck ratchet, formatting/lint/changed-line gates and source contracts pass. The unchanged host source/tests pass152 tests on the immediate test-consolidation predecessor.
Production serialized-RPC stress rejects4,612,861 bytes before; after it admits2,030,250 bytes and paginates all2,402 IDs without splitting groups. This synthetic SQLite control is separate from genuine CLI history; no live socket1013 close is claimed.
Fresh genuine-data pipeline and hidden430×900 rendered pair/notice proof pass. Task-owned capture processes/app exited and provider/CDP/proof ports closed.
The fresh watcher emits snapshot41 → appended1 → replacement41. A separately labeled appended-delivery replay uses the genuine final pair; it is a controlled frame, not this run's actual watcher delivery. Direct RPC/JSON checks and hidden production-component captures do not establish a physical device, relay/WebSocket or SSH run.
Review
The branch now includes main
94f5669in6b8cafb. All 21 reviewed files and all 20 product files covered by the earlier OpenCode evidence keep their exact preceding postimages; every other main file is preserved. Independent full-source and artifact reviews pass. Fresh Node/CLI/web and production mobile TypeScript 6 typechecks pass, together with 105 focused tests across seven desktop suites. The desktop runner excluded the requested mobile pair-window test, so it is not counted. The initial mobile typecheck lacked ordinary generated assets; the five existing generators completed successfully, and only that failed typecheck was repeated. Earlier full quality, installed OpenCode 2.0.16 and hidden-renderer receipts remain tied to their tested sources. Fresh public CI covers this composition before merge. The broader work in #13287 remains open.Agent skill upstream boundary
Notes
The executed platform is macOS. Existing optional-field fallback and frame types support mixed-version peers; host execution and folder-workspace behavior are preserved. Linux/Windows rendering, physical iOS/Android and SSH pairing were not exercised.
Checklist