Repository navigation
fix(frontend): validate persisted state and own UI lifecycles - #48
Merged
Merged
Conversation
added 9 commits
October 4, 2026 03:27
Implement the admitted FE-A05 subset on exact ca9edde. Decode the existing unversioned record from unknown, checking nested actions, opaque nonblank IDs, nonnegative safe millisecond timestamps, the existing 32-entry bound and valid cursor relationships. Keep the omitted-position legacy fallback. Reject invalid/oversized records as a whole, log UNDO_STORE_LOAD_FAILED, and preserve their bytes. Unavailable or rejected storage disables writes for that store instance, including later in-memory pushes and clear; skipped persistence reports UNDO_STORE_SAVE_FAILED. No schema migration or recovery writes. Move unchanged operation bodies into an injected store factory, with per-store callback ownership. Keep the app singleton and exported action/entry types. No timeline or Git cleanup implementation changes. Rejected entries never reach an aging callback; fresh user actions retain their existing in-memory behavior. Validation on pinned Node 24.12.0/npm 11.6.2: - 35 focused decoder/store tests and 582 aggregate frontend tests pass. - Full lint, TypeScript, production build, staged critical/a11y/traceability gates and diff whitespace pass. - Existing stale Browserslist data notice remains; no dependency changes. - Source comparison confirms all operation bodies preserved after logger and constant renaming. Tests use recording callbacks, never permanent Git deletion. The evidence report records exact policy and limits. This does not qualify GUI, native/model execution, cold browser reopen, other persistence owners or full FE-A05 schema/version migration. Separate scanner/fixture branches are preserved; write sets are disjoint. Independent review remains required before integration.
Resolve FE-A02 stale animation completions by cancelling and settling superseded navigation, reset, and immediate breadcrumb timers. Share persistence subscriptions and debounce timers across independently released enablePersistence owners. Validation: 15 focused timer tests; 562 aggregate frontend tests; TypeScript, full lint, production build, staged critical/a11y/traceability, whitespace pass. Regression fails against ca9 before repair. GUI/native/model checks not executed. Base: ca9edde (full ancestry retained).
Prevent deferred device initialization from creating refresh intervals or applying read results after component destruction. Share initialization across duplicate starts and observe active/late failures with a mount-owned lifecycle helper. Keep backend requests settling and preserve existing configuration policies. Validation: 10 focused lifecycle tests, actual-script before/after deferred probe, 557 aggregate frontend tests, full lint, TypeScript, production build, staged critical/a11y/traceability and whitespace pass. GUI/native/model checks not run. Base: ca9edde (full ancestry retained).
Return synchronous cleanup from application and package graph mounts. Preserve six listener identities/options and existing registrar exports. The shared graph mount owner guards late definition application and observes live/late rejection. Validation: 12 tests execute actual component mount callbacks plus the existing listener identity test; both synchronous-cleanup regressions fail against ca9. All 559 frontend tests, full lint, TypeScript, production build, staged critical/a11y/traceability and whitespace pass. GUI/native/model checks not run. Base: ca9edde (full ancestry retained).
Record exact ancestry-preserving composition of accepted undo, view, device and graph mount repairs. Combined focused 73 and aggregate 619 frontend tests pass, as do full lint, TypeScript, build, explicit ca9-to-composition critical/a11y/ traceability and whitespace checks. Preserve native/visual/IPC/model limits. Post-base fixture/scanner branches are excluded; no PR44/47 ref changes. Record a controlled malformed-view-record probe as the next bounded candidate, without implementing or accepting additional persistence policy.
Decode the existing unversioned view record before applying any owned field. Retain malformed/unavailable bytes and close instance writeback on rejection. Initialize before the first persistence scope; validate later enables without replacing local edits. Suppress synchronous restore-time writes and recheck storage/outgoing values before each write. Preserve shared timer ownership, partial legacy records, serialization and existing public action return types. No schema migration, recovery writes or new product limits are introduced. Validation: 55 focused cases, including all 15 accepted view lifecycle tests; 659 aggregate frontend tests; full lint, TypeScript, build, critical/a11y/ traceability (7 paths, 0 mapped impacts), whitespace pass. Two defect regressions fail against exact bb45; two restart compatibility controls pass on base/repair. GUI, actual native IPC, model and real cold-reopen checks were not executed. Base: bb45b81; accepted source ancestry retained.
Qualify the exact fd1ace frontend lineage composed with native/scanner integration 41701d6. Preserve both accepted parents and every admitted blob; no path overlaps or conflict resolution. Preserve exact scanner 5590b4a and unchanged Svelte markup. Combined 659 frontend tests and 63 tooling tests pass; full lint, TypeScript, build, Rust formatting, critical/a11y (27 scanner tests), whitespace and explicit traceability pass. Common ca9 range: 34 paths/0 impacts; native417 range: 24/0; historical main4938 range: 251/1 with declared legacy-map adoption. Qualified source: 5eb2f08, tree 2826057. This successor adds evidence only. No native/WebKit/model/release execution or PR44/47 updates/bot requests.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (36)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThis change adds lifecycle helpers for device configuration and workflow graph mounts. It also updates view-store animation and persistence behavior, adds a persisted undo store, coordinates Pumas registry preparation in tests, and adds qualification reports. ChangesPumas registry test startup
Device configuration lifecycle
Workflow graph mount lifecycle
View navigation and persistence
Undo history persistence
Source composition qualification reports
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant DeviceConfig
participant Lifecycle
participant DeviceAPI
participant RefreshScope
DeviceConfig->>Lifecycle: start initialization
Lifecycle->>DeviceAPI: list devices
Lifecycle->>RefreshScope: start refresh
Lifecycle->>DeviceAPI: load embedding mode
DeviceConfig->>Lifecycle: stop on destruction
Lifecycle->>RefreshScope: run cleanup
sequenceDiagram
participant WorkflowGraph
participant MountScope
participant WindowListeners
participant DefinitionLoader
participant GraphStore
WorkflowGraph->>MountScope: create mount scope
MountScope->>WindowListeners: register handlers
MountScope->>DefinitionLoader: load definitions
DefinitionLoader-->>MountScope: return definitions
MountScope->>GraphStore: apply definitions while active
WorkflowGraph->>MountScope: stop on cleanup
MountScope->>WindowListeners: remove handlers
sequenceDiagram
participant ViewStore
participant Storage
participant PersistenceScope
ViewStore->>PersistenceScope: enable persistence
PersistenceScope->>Storage: read and validate stored record
PersistenceScope-->>ViewStore: restore valid fields
ViewStore->>PersistenceScope: publish state change
PersistenceScope->>Storage: write validated state after debounce
ViewStore->>PersistenceScope: release persistence handle
Merge Risk: ⚪ Minimal · up to The change adds lifecycle ownership and validation for persisted view and undo state, plus test-only registry preparation. No concrete merge-blocking problem was found. Before merging, wait for the exact-head hosted workflows to finish. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change strengthens saved-state validation and limits work after teardown or navigation supersession. No introduced or worsened security issue was established. Permanent history deletion already existed, but trust in saved undo records and recovery after deletion failures remain incompletely established. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 48.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 68 functions across 24 files. (12 skipped: 12 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Owner
Author
|
@coderabbitai full review Please review this main-relative36-file change at current head ee91e5d (tree5080b74e82e5ef7bb62741d7fc03cc4754ea1f4a) against main b84d7ec. No earlier review of PR48 has been requested. The accepted frontend composition validates persisted records without overwriting rejected bytes and owns graph listeners, DeviceConfig initialization, animation and persistence lifetimes. The successor adds test-only shared registry warm-up while preserving existing assertions, production code, dependencies and parallel test bodies. Independent review accepted the exact composition and source; the accepted tree passed456 embedded tests and the required258+168 npm tests (one inherited benchmark ignore). Prior unchanged frontend/tooling coverage is659+63. Please check correctness and regression coverage without treating warm-registry tests as production cold-start qualification. Current Pumas has an upstream WAL-startup repair absent from this older pin, and the original hosted failing SQL statement is unproven. Exact-head hosted Quality37187388027, Headless37187388013 and Runtime37187388014 are still running; no aggregate pass is claimed yet. Genuine GUI, native IPC/hosts and model acceptance remain outside scope. Use one included review only; no paid review or upgrade. Report actual source range/file coverage and any actionable findings. |
✅ Action performedFull review finished. |
MrScripty
marked this pull request as ready for review
October 4, 2026 08:14
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Accepted candidate and registry-fixture qualification
Current published head: ee91e5d. Exact accepted tree: 5080b74e82e5ef7bb62741d7fc03cc4754ea1f4a. The original ea0378 frontend composition and all its ancestry are retained. Connector publication recreated only commit metadata: source78d35f9e maps to150efee067ed5bfb6773d0dc9cff9bbdc63fb4c6 (tree dfe5deaa446530866a0ea2075442ff6f1e1545f1, sole parent ea0378); documentation7cb06b73 maps to this head (sole parent150efee0). Both trees and ordered parents were verified before the non-force ref update.
Independent source review accepted the bounded test-fixture warm-up. All27 fixture builder sites are test-only, and reversing those references plus the cfg(test) module declaration restores all nine existing files byte-for-byte. Existing assertions, dependencies, production behavior and workflow gates are preserved. The once-only registry preparation retains a connection; API construction and test bodies still run concurrently, and initialization errors still fail tests.
Exact accepted-tree native qualification subsequently passed all456 embedded-runtime tests with zero failures/ignored. The required npm test gate passed258 node-engine tests plus168 workflow-nodes tests, with one inherited benchmark ignore. Existing official ORT1.24.2 and dedicated disposable registry paths were used. The earlier resource-limited attempt ran no tests; only the later complete run counts as native qualification. Earlier frontend composition evidence remains659 frontend and63 tooling passes, with exact blob/mode and disjoint-composition checks.
This is warm-registry fixture stabilization, not production cold-start qualification. The retained cold-open experiment reproduced the old pinned Pumas registry race, but the hosted failing SQL statement is unproven. Current Pumas already contains an independently reviewed WAL-startup repair absent from Pantograph's older pin. No dependency upgrade is included, and warm tests do not replace an eventual reviewed pin upgrade and retained cold-open tests. Repair source and historical investigation.
Current hosted and external gates
All three exact-head workflows completed successfully: Quality37187388027, Headless37187388013, and Runtime37187388014. The old ea0378 Quality failure remains historical evidence. The completed CodeRabbit full review selected all36 main-relative files through exact ee91e5d, generated no actionable findings, and left no review threads. The48.53% docstring warning is not a failing repository CI gate. Its noted pre-existing undo-history deletion authorization/recovery limitations remain a separate hardening scope; this change does not claim to resolve them.
Genuine GUI/WebKit, physical input, real browser cold reopen, current-tree native IPC/host/package/model loading and inference remain outside the executed local qualification. No release or end-to-end product completion is claimed. Exact hosted and external-review gates are now complete; this source is eligible for the authorized integration transition.
Earlier frontend composition and hosted failure
Malformed persisted undo/view records could enter frontend state or be overwritten during startup, and UI work could outlive teardown or newer navigation. This change validates owned records before application/writeback, preserves rejected bytes, and owns graph listeners, DeviceConfig initialization, navigation animations and persistence timers.
Valid legacy view records and local edits between persistence scopes remain supported. The view record retains its current unversioned format; no schema migration, recovery writes or new product limits are introduced.
Exact candidate
ea0378defadebb8665bc964c352166f0fbb15bb7f03ad681e6952266f48a1907e07cb39b05e00328qualification/all-source-accepted-candidatemainatb84d7ec49773ab306432b393b735e4aae2e027b1The candidate preserves both accepted lineages: frontend
fd1ace640ddb51ef906991a6d658707d5ef034f5and native/scanner41701d68fb3748f0b422b247427e4ca6d51ccbd5. Native/scanner changes are already on main and remain unchanged. The PR adds 24 accepted frontend paths plus one combined qualification report. Composition had no path intersections or conflicts, and the exact scanner gate is preserved.Validation
Checks ran on composition
5eb2f087e36d1dec1377f78bed5b7afd067c282e, tree2826057b536babcca9aeae7260fd25d46b50a9a3; the final successor only adds the report.npm run test:frontend: 659 passed; zero failures/skips/cancellations.node --test scripts/*.test.mjs: 63 passed.npm run lint:full,npm run typecheck,npm run build, andcargo fmt --all -- --check: passed.Full qualification evidence
Review and limits
Independent composition review accepted this exact candidate. All 24 frontend blob identities and modes match accepted fd1ace6; all 10 native/scanner paths match reviewed 41701d6. The path sets are disjoint and the only additional file is the combined qualification report. Main b84d has the same source tree as 41701d. Fresh scanner/accessibility, traceability and whitespace checks passed; prior 659 frontend and 63 tooling results retain their unchanged-source scope. Final integration remains gated on exact hosted CI and external review.
These are frontend/tooling/static results. Genuine GUI/WebKit, actual native IPC, native package loading, C#/BEAM hosts, real browser cold reopen and model loading/inference were not executed here. Controlled GUI-wrapper fixtures do not establish genuine GUI acceptance. The parent reports GUI helper-layout preflight stopped on a read-only filesystem without a workaround; the CDN restriction remains.
Current hosted gate
Exact-head Quality Gates 37181984567 failed in embedded runtime unit tests: owner_api_projects_diffusers_package_facts_without_paths could not initialize Pumas because SQLite reported DatabaseBusy / database is locked at pumas_dispatch_package_facts.rs:340. The suite reported 454 passed and 1 failed. Other Quality jobs, including frontend tests, strict Clippy, dependency audit and traceability, passed. This failure remains under investigation; it is not waived or labeled harmless.
Runtime Separation 37181984577 passed; Headless Workflow Contract 37181984580 was still running at the latest check. No current-head aggregate or native-workflow pass is claimed. External review has not yet been requested; the coordinated repository allowance opens no earlier than 06:34 UTC on 2026-10-04.
This remains a draft. The previous PR44/47 source and metadata are not changed by this update.
Summary by CodeRabbit