perf(V2): bound transcript memory - #648
Conversation
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review, pass 1: changes required
Merge is blocked pending correction of these findings:
- Client replacement currently emits full instance-destruction semantics, which can discard active selection, drafts, expansion state, and transcript state; pending permission/question synchronization can also apply stale responses from the replaced client.
- Subagent transcript loading can start before the child session exists and does not safely share request ownership across multiple mounted consumers.
- Running native refresh/session-status continuations can survive instance teardown and recreate or mutate state after removal.
- Permission resources, task output, todo output, and apply-patch copy paths can hide or make authoritative content inaccessible while still allowing user decisions.
- Retained-size accounting misses Map/Set contents and can double-count shared ArrayBuffer storage.
- Info-view back navigation, same-diff permission identity, search partiality/revealability, descendant totals, and exact task-limit signaling need correction.
I am applying the corrections now and will publish another gatekeeper pass after validation. The PR remains Draft.
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review, pass 2: changes still required
The first-pass findings are corrected and the combined branch passes UI typecheck, production build, and 71 focused tests. A second full review found additional blockers:
- Streaming mutations can repeatedly cancel retained-size measurement, leaving active growth unaccounted; LRU touches before accounting are also lost.
- Client/reconnect invalidation must clear old load errors, force authoritative reconciliation, and invalidate request sync even when the shared V2 proxy/client identity is unchanged.
- Shared-load lease completion can overwrite newer authority, and mounted child tasks do not react to loaded-state invalidation.
- Permission keyboard shortcuts can approve while another button is focused; event/reply continuations need current instance/client/request authority.
- Large permission review needs bounded processing and a non-clipboard access path; the modal also needs keyboard focus containment.
- Search must continue after marking a truncated part partial; aggregate-truncated diffs need fallback detection; truncated child steps and Status-tab todos need complete-content access.
- Child deletion must recompute ancestor totals, Info restore must retain its conversation target, and the new runnable tests must be added to CI.
I am applying pass-2 corrections now. The PR remains Draft and blocked until a subsequent pass reports no findings.
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review, pass 3: changes still required
Pass-2 corrections now pass full UI/Electron typecheck, 28 standard focused tests, and 63 browser-condition tests. The third full review found remaining interactions:
- External forced transcript reloads can trigger the mounted loader and abort the caller-owned refresh; reload ownership must be single-source.
- Evicting an infinite retained-size estimate can poison LRU arithmetic with NaN.
- Long sessions must follow native V2 message pagination so eviction/reload restores complete history.
- Same-client reconnect authority is still missing from session-info and question reply continuations; permission event merging/reply completion also needs stable request semantics.
- Search indexing must match actually rendered task/apply-patch/output regions and budgets.
- Measurement and async Markdown/diff cache work must not resurrect stale data after mutation/eviction.
- The CI cleanup correctly removes Tauri, but must retain the independent Windows server spawn test.
- Permission shortcuts inside the active modal must remain usable without duplicate document handling.
- Tool-call documentation still describes the old truncation/copy behavior.
I am applying pass-3 corrections and will repeat the gatekeeper review. The PR remains Draft and blocked.
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review, pass 4: changes still required
Pass-3 corrections typecheck successfully and fix full V2 message pagination, cache resurrection, LRU Infinity handling, reconnect authority, search alignment, and Windows Node validation. Pass 4 found remaining blockers:
- Authoritative message replacement must purge omitted message render caches; failed/empty accounting must not retain pending LRU touches.
- Tool search/render parity still has gaps for fallback output, diagnostics, collection budgets, task truncation, and input copy format.
- Shared pending-request synchronization needs consumer leases so one timeout cannot cancel reconnect recovery; several same-client reconnect paths still need authority/version checks and delta-buffer cleanup.
- The standard CI partition needs forced exit for tests that retain UI handles.
- Removing Tauri setup from the caller is insufficient while the reusable build workflow still enables Tauri jobs; the DEV-v2 PR path must disable those jobs while retaining the Windows Node spawn test.
- Migration documentation must retain unresolved security work not addressed by this UI-only PR and describe Tauri removal narrowly and accurately.
I am applying pass-4 corrections now and will run another full gatekeeper pass. The PR remains Draft and blocked.
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review, post-rebase pass: changes still required
The branch is now rebased onto DEV-v2@c66aac79, which removed the duplicate Rust-native event transport and resolved the prior server/service findings. Redundant reconnect buffering and Tauri workflow changes were removed. Current validation passes full typecheck plus 140 focused tests.
The remaining findings are limited to this PR's renderer-memory scope:
- Per-session derived render/display caches must be included in transcript accounting now that the old five-session cap is gone.
- Structured input and �pply_patch still perform a complete eager formatting/join before truncation or copy.
- Search parity has two remaining empty-diff/error-budget gaps.
- Long single task titles need a complete copy path; diagnostic dedupe, empty copy buttons, and live-region review semantics need small corrections.
I am applying this final scoped correction set and will repeat the review. The PR remains Draft.
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review, final scoped pass: two axes clear, corrections remain
The rebased branch now has no async-authority finding and no CI/documentation finding. DEV-v2 authorization is included, upstream Tauri validation is preserved, and the current focused suites/typecheck pass.
Remaining renderer-memory findings:
- The permission modal portal must close with its owning hidden instance, and complete permission reviews must not mount multi-megabyte content while collapsed.
- Permission title/resource checks, todo normalization, apply-patch file selection, and transcript measurement still create some eager full-collection snapshots.
- Derived display caches and unknown pending parts need complete accounting/cleanup, and touching an inactive transcript must refresh accounting after render-cache growth.
- Empty child-task scan windows must still expose truncation/full-copy, and search limits must match actually renderable structured output/prompt regions.
I am applying this final renderer/memory correction set. Async and CI/docs gates remain clear; the PR stays Draft until the next pass reports no findings.
2910e88 to
bb39dab
Compare
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review: scoped implementation clear
The branch is now rebased onto DEV-v2@c66aac79 and the duplicate Rust-native Tauri event transport remains removed. The PR was re-scoped to the renderer-memory issue: bounded DOM/eager processing, one global transcript/cache budget, lossless bounded V2 reload, and complete teardown cleanup. Search and permission-policy expansions were removed.
Final validation passes full UI/Electron typecheck, production UI build, 248 standard UI tests, 71 browser-condition UI tests, 118 Electron native tests, and diff checks. The final child-session discovery race was corrected and its focused tests pass.
No scoped gatekeeper finding remains. The PR stays Draft only for the outstanding 24-hour WebKit renderer-memory soak.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32199549214 Artifacts expire in 7 days.
|
1 similar comment
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32199549214 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32202206283 Artifacts expire in 7 days.
|
1 similar comment
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32202206283 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32215067376 Artifacts expire in 7 days.
|
1 similar comment
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32215067376 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32226248908 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32228465460 Artifacts expire in 7 days.
|
1 similar comment
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32228465460 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/32257947226 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/33347539512 Artifacts expire in 7 days.
|
Bring PR NeuralNomadsAI#648 onto DEV-v2 at d44d7f6 without changing its bounded-transcript implementation. The new base replaces the isolated developer-run launcher with the persistent in-process Electron and Tauri Developer Mode contract, project-local OpenCode automation plugin, and restart-aware desktop integration. The transcript-memory diff remains byte-for-byte unchanged across its 75 files. Validation covers root typechecks, 303 standard UI tests, 149 browser UI tests, 178 Electron native tests, 369 server tests with two platform skips, 131 Rust tests, desktop packaging invariants, and Tauri dev preparation.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/33376566378 Artifacts expire in 7 days.
|
1 similar comment
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/33376566378 Artifacts expire in 7 days.
|
Advance PR NeuralNomadsAI#648 to DEV-v2 at e0d9420. The updated base routes WSL-hosted Developer Mode calls through Windows interop and clears failed Electron relaunch intent so later ordinary quits cannot restart unexpectedly. The bounded-transcript diff remains byte-for-byte unchanged across all 75 files. Root typechecks, 179 Electron native tests, and the server suite with 369 passes and two platform skips validate the affected desktop automation paths.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/33379338818 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/33382373635 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/33821702389 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/33821702389 Artifacts expire in 7 days.
|
1 similar comment
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/33821702389 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/33828864539 Artifacts expire in 7 days.
|
Reconnect the bounded transcript-memory work directly to dev after the native V2 migration landed through NeuralNomadsAI#647. Apply only the NeuralNomadsAI#648 content delta on top of dev@52f0e629, avoiding the now-squashed DEV-v2 history. Bound transcript rendering and derived caches, retain complete content behind lazy copy paths, and coordinate inactive transcripts under a byte-aware 64 MiB LRU. Protect visible, loading, live, and interruption-blocked sessions while fully purging evicted instance/session state. Restore evicted transcripts through bounded 200-message V2 windows with opaque cursor authority, stale-request fencing, revision retries, committed viewport movement, and exhaustive task-copy behavior. Regression coverage exercises cache replacement, pagination, reconnect authority, permission safety, and eviction reloads across the supported desktop clients.
Set both Windows LOCALAPPDATA and POSIX XDG_RUNTIME_DIR while exercising the Developer Mode bridge registry, and derive the asserted directory through the production resolver. This prevents Linux CI from writing registrations under the runner home while the test creates stale entries under an unrelated temporary path. Restore both environment variables after each test so the suite remains isolated. Validated with 20 repeated targeted runs, the server typecheck, and the complete server test suite.
Route manual compaction through the same per-session admission chain as prompts, commands, and shells so a compact request cannot overtake already admitted work. Resolve the current client and session only when queued work starts, and make the /compact command use that shared action. Preserve native SessionInfo metadata when paginated project inventory responses reconcile duplicate sessions that omit the optional field. This keeps durable V2 metadata intact across root and descendant catalog passes. Add regression coverage for prompt-before-compaction ordering and metadata retention during cursor-based inventory hydration. Validated with the focused 67-test browser suite, UI typechecking, the runnable UI matrices, and the UI build.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/33828864539 Artifacts expire in 7 days.
|
66de74c to
f4f0256
Compare
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/33828864539 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/33866523806 Artifacts expire in 7 days.
|
Keep technical grouping cache signatures aligned with the existing 200-part message render window. Very large tool transcripts no longer rebuild unbounded group and visibility strings during reactive invalidation, avoiding UI stalls and transient memory growth while preserving the rendered head and final response. Extract the shared head/tail part selection into record-display-cache so rendering and signature invalidation cannot drift. Extend the message store regression test to verify the shared bounded selection. Validated with the targeted message window, render-memory, transcript LRU, and instance-store tests plus the UI TypeScript check.
pascalandr
left a comment
There was a problem hiding this comment.
Revue gatekeeper
Finding bloquant — corrigé
[P1] Borner la signature réactive des groupes techniques à la fenêtre réellement rendue
technicalGroupingSignature() parcourait deux fois record.partIds et concaténait une signature pour toutes les parties du message, alors que le rendu est volontairement limité à 200 parties. Une réponse contenant un très grand nombre de parties pouvait donc encore provoquer un travail synchrone et des allocations non bornés à chaque invalidation réactive, en contradiction avec l'objectif mémoire de cette PR.
Corrigé par f9c476f6 : le rendu et la signature partagent désormais la même sélection tête/fin bornée à 200 parties via getRecordDisplayPartIds(). Le test de régression vérifie la borne et la conservation du début et de la réponse finale.
Validations ciblées : 44/44 tests réussis, plus typecheck UI réussi.
Audit OpenCode V2
Contrat publié vérifié jusqu'à @opencode-ai/client@0.0.0-beta-19157 et branche upstream beta@7a4ad68a. Les évolutions postérieures à beta-18999 concernent notamment l'état provider des compactions, la robustesse des refreshs Solid et le contrat worktree natif ; aucune ne révèle d'autre écart fonctionnel pertinent pour le budget transcript, les fenêtres bidirectionnelles ou l'admission par session de cette PR.
Verdict après correction : aucun autre finding bloquant.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/33980613880 Artifacts expire in 7 days.
|
Bring the merged binary compatibility, startup shell, Tauri window capture, and Linux automation registry fixes into this PR without changing its feature scope. Resolve the automation test overlap by retaining per-test registry isolation and the upstream temporary-root assertion. Windows and Linux registry variables are restored, and WSL discovery is disabled only inside isolated test fixtures so tests cannot touch the real desktop registry. Validation: all eight automation plugin tests pass; server and UI TypeScript checks pass; git diff --check passes. This conflict-resolution commit is not a final native-release gatekeeper approval.
Merge dev@81d536f6 into PR NeuralNomadsAI#648 while preserving the 64 MiB transcript LRU, 200-message replaceable windows, committed pagination results, and multi-consumer cancellation. Combine the empty older-page handling and staged native undo from the current V2 baseline with the bounded paging flow. Track the window identity installed by a successful load so the request keeps authority long enough to return its committed result, while stale loads remain fenced. Keep the newer rendering, shell startup, diagnostics, Turkish localization, and browser regression coverage from dev without taking commits from PR NeuralNomadsAI#649. The final PR file set remains unchanged at 78 paths. Validated with UI, Electron, and server typechecks; 117 targeted browser-conditioned store/action tests; the full 412-test server suite (410 pass, 2 platform skips); 189 Electron native tests; 24 Chromium rendering tests; 160 Tauri Windows tests; desktop packaging invariants; and OpenCode compatibility tests. Audited published @opencode-ai/client beta-19271 against beta-19157 without finding a required dependency update for this PR.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/34414551425 Artifacts expire in 7 days.
|
Integrate dev@ea37f97b after the global-session, nested-scroll, and selective-pruning fixes while preserving the bounded transcript-memory implementation. The merge is conflict-free and keeps the PR history intact for exact-head CI validation.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/34486377502 Artifacts expire in 7 days.
|
Summary
latest/historywindows with opaque cursor authority, stale-request fencing, and preserved scroll stateThis supersedes #626 without modifying or closing that historical PR and closes #553.
Reconnected to
devAfter #647 merged the native V2 migration, this branch was rebuilt directly on
dev@52f0e629; PR HEAD isf4f02563(3,078+/651-, 78 files).dev; the functional patch-id is identical to the previously validated branchdeveloper-runlauncher, legacy shims, or the former Developer Automation settings surfaceFinal OpenCode V2 audit
@opencode-ai/clientbeta contract (beta-18999)/compactthrough the same per-session admission chain as prompts, commands, and shellsSessionInfo.metadataacross duplicate paginated inventory responses that omit the optional fieldloadMore({ all: true })andsession.evict()do not provide a global 64 MiB LRU or replaceable bidirectional windowsValidation
310/310156/156189/189376passed,2platform skips3/310/10in isolation after intentional local cross-suite contentiongit diff --check, and conflict-marker scanGitHub CI for
f4f02563is pending after reconnecting the PR todev.