ui(data-mode): migrate legacy harness storage to v2 - #1040
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe data-mode storage key changes from v1 to v2. Runtime logic migrates recognized legacy state, preserves v1 data, avoids redundant writes, and excludes host-owned keys. Unit and E2E tests now validate v2 behavior and share the versioned key. ChangesData mode migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant E2ETest
participant DataModeProvider
participant readStoredDataMode
participant localStorage
E2ETest->>localStorage: initialize v2 data-mode state
DataModeProvider->>readStoredDataMode: read fallback and persistence settings
readStoredDataMode->>localStorage: read v2 and legacy v1 values
readStoredDataMode->>localStorage: seed v2 from fallback when legacy state is recognized
readStoredDataMode-->>DataModeProvider: return selected data mode
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
Contributor
|
This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/mesh-llm-ui/e2e/configuration/schema-controls.spec.ts (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the data-mode storage key literal into a shared e2e constant.
The
mesh-llm-ui-preview:data-mode:v2string is independently defined or hardcoded in four separate spec files; this PR had to edit all four in lockstep to bump v1→v2, which is exactly the kind of drift risk a shared constant avoids for the next version bump.
crates/mesh-llm-ui/e2e/configuration/schema-controls.spec.ts#L4-L4: import the key from a shared e2e helper module instead of a local const.crates/mesh-llm-ui/e2e/plugins/web-ui-exemplar.live.spec.ts#L99-L99: pass the key viaaddInitScriptargs from the shared constant (as the other specs do) instead of hardcoding the literal inline.crates/mesh-llm-ui/e2e/smoke/chat-mobile.spec.ts#L3-L3: import the key from the same shared helper module.crates/mesh-llm-ui/e2e/smoke/live-parity.spec.ts#L3-L3: import the key from the same shared helper module.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/mesh-llm-ui/e2e/configuration/schema-controls.spec.ts` at line 4, Centralize the data-mode storage key in a shared e2e helper constant, then update all four specs to reuse it: import the constant in schema-controls.spec.ts, chat-mobile.spec.ts, and live-parity.spec.ts instead of local literals, and pass it through addInitScript arguments in web-ui-exemplar.live.spec.ts instead of hardcoding the key.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/mesh-llm-ui/e2e/configuration/schema-controls.spec.ts`:
- Line 4: Centralize the data-mode storage key in a shared e2e helper constant,
then update all four specs to reuse it: import the constant in
schema-controls.spec.ts, chat-mobile.spec.ts, and live-parity.spec.ts instead of
local literals, and pass it through addInitScript arguments in
web-ui-exemplar.live.spec.ts instead of hardcoding the key.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1660a7c2-2f30-4ffd-a653-1aebbb03f904
📒 Files selected for processing (6)
crates/mesh-llm-ui/e2e/configuration/schema-controls.spec.tscrates/mesh-llm-ui/e2e/plugins/web-ui-exemplar.live.spec.tscrates/mesh-llm-ui/e2e/smoke/chat-mobile.spec.tscrates/mesh-llm-ui/e2e/smoke/live-parity.spec.tscrates/mesh-llm-ui/src/lib/data-mode/DataModeContext.test.tsxcrates/mesh-llm-ui/src/lib/data-mode/DataModeContext.tsx
Collaborator
Author
|
@CodeRabbit review |
Contributor
✅ Action performedReview finished.
|
ndizazzo
force-pushed
the
codex/issue-633-data-mode-migration
branch
from
July 22, 2026 01:25
6979b09 to
0610556
Compare
michaelneale
added a commit
that referenced
this pull request
Jul 22, 2026
…ngram * origin/main: Add GLM DSA correctness trace reporting (#1033) Add GLM DSA package contract validation (#1032) Add direct quant layer package tooling (#1031) Lock split topology placement (#1050) fix nightly stability Qwen thinking ui(data-mode): migrate legacy harness storage to v2 (#1040) fix: Windows Vulkan runtime dependencies (#1046) test: validate MI300 runtime selection and placement (#1045) fix: support ROCm RDNA APU kernels safely (#1044) fix: ROCm GPU enumeration (#1039) Document public packaging installation channels Fix Windows autoupdate import (#1030) Update llama.cpp upstream pin
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.
Summary
v1tov2so legacyharnessdefaults are not mistaken for explicit user choicesv2from the current environment default only during a valid app-key upgrade, retainv1for downgrade compatibility, and avoid repeated writesv2and add migration/error/idempotency regression coverageCloses #633
Validation
DataModeProvidertests: 11 passedpnpm run lintpnpm run typecheckjust website-buildjust test-all(all 10 phases, including 1,700 host-runtime tests and Playwright smoke tests)Notes
The first full gate encountered one load-sensitive timeout in an unrelated mesh-visualization test; that test passed immediately in isolation. A subsequent full run passed the complete UI suite. The next run exposed missing website dependencies in the fresh worktree; after installing the lockfile dependencies, the website build and a final complete
just test-allboth passed.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores