Carry workspace comparisons across releases - #2852
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a3f339a81
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const build = typeof trial.build === 'string' && trial.build.trim() ? trial.build.trim().slice(0, 256) : null | ||
| trials.value.push({ ...trial, build, ease, note: trial.note.slice(0, 2000), recordedAt: new Date().toISOString() }) | ||
| if (typeof trial.theme !== 'string' || !trial.theme || trial.theme.length > 64 || typeof trial.note !== 'string') return false | ||
| trials.value.push({ ...trial, id: crypto.randomUUID(), frontendBuild: frontendBuildIdentity, build, ease, note: trial.note.slice(0, 2000), recordedAt: new Date().toISOString() }) |
There was a problem hiding this comment.
When Taskdeck is opened through the documented plain-HTTP LAN path, the origin is not a secure context; docs/platform/LAN_DEVICE_ACCESS_GUIDE.md:290-294 already identifies an unguarded crypto.randomUUID() call as a hard failure in that environment. This newly added call therefore throws before the observation is appended, and the submit path has no error state, leaving the user without a saved observation or recovery message. Guard this call with an ID-generation fallback and cover the supported LAN mode.
AGENTS.md reference: frontend/AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
|
Independent Luna review found one HIGH: clearing observations while a selected file was being read could allow that import to restore the cleared notes. Fixed in 2fb2a4a: Clear changes the store reset version, synchronously cancels the component read generation, and invalidates pending store imports. Both file-read and legacy-hash cancellation regressions pass (15 targeted tests); production build passes. Bounded exact-fix review is CLEAN with no new HIGH/CRITICAL. Current head ba3c56a also integrates main856172dfa's unrelated Paper Review focus fix. Independent interaction review is clean; seven focus/context tests and sixteen focus/private-source tests passed in the stacked overhaul integrations. New hosted CI must finish before merge. The preceding comparison full frontend run passed 6,282 tests with three existing skips; final Chromium comparison journey and mobile table accessibility check passed before this narrow cancellation fix. No statistical A/B or deployment assertion is made. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba3c56a3ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const bytes = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(JSON.stringify(value))) | ||
| value.id = `legacy-${Array.from(new Uint8Array(bytes), x => x.toString(16).padStart(2, '0')).join('')}` |
There was a problem hiding this comment.
On the documented plain-HTTP LAN origin, crypto.subtle is unavailable because SubtleCrypto requires a secure context, so importing any supported version-2 export throws here instead of carrying the observations forward. Beyond the already reported randomUUID() failure, this is a separate new Web Crypto dependency in the legacy-import path; fixing ID generation alone will therefore leave version-2 imports broken in that supported environment.
Useful? React with 👍 / 👎.
| const env = loadEnv(mode, process.cwd(), 'VITE_') | ||
|
|
||
| return { | ||
| define: { __TASKDECK_FRONTEND_BUILD__: JSON.stringify(command === 'build' ? fingerprintFrontend(process.cwd(), mode, env) : null) }, |
There was a problem hiding this comment.
The fingerprint only receives mode and loaded VITE_ values, so Vite CLI options that alter the generated frontend are omitted. In particular, .github/workflows/pages-frontend.yml:49 builds with --base /Taskdeck/ while the release workflows use the default base, yet both builds receive the same identity when their source, environment, and Node version match; imported observations can consequently be grouped as the same frontend build even though their generated asset URLs and routing differ. Derive the identity from the resolved build configuration or otherwise include these output-affecting options.
Useful? React with 👍 / 👎.
|
Retained comparison follow-through for #2808, triaged once from automatic review of PR #2852:
These remain concrete follow-up work under the umbrella. The current PR's independent HIGH clear/import race is fixed and its bounded fix review is clean. No further review-fix cascade is added to that PR; no claim that the whole overhaul is complete. |
…omparisons # Conflicts: # docs/IMPLEMENTATION_MASTERPLAN.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98302b4bd6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const groups = computed(() => { | ||
| const result = new Map<string, { key: string; trial: WorkspaceTrial; count: number; completed: number; blocked: number; rated: number; easeTotal: number }>() | ||
| for (const trial of trials.value) { | ||
| const key = JSON.stringify([trial.scenario, trial.experience, trial.presentation, trial.theme, trial.build, trial.frontendBuild]) |
There was a problem hiding this comment.
When the user records multiple trials with the auto theme while the OS color scheme changes, every observation retains theme === 'auto', so this key merges light and night trials into one aggregate even though paperThemeStore.ts:65-66 renders them differently. This makes the newly added completion and ease statistics combine non-comparable visual conditions; record the resolved appearance with each trial or otherwise include it in the grouping identity.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d66d98427f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| else throw new Error(`Unsupported frontend input: ${name}`) | ||
| } | ||
| } | ||
| for (const folder of ['src', 'public', 'build']) directory(folder) |
There was a problem hiding this comment.
Restrict the fingerprint to output-affecting frontend inputs. Recursing over all of src and build also hashes src/tests/** and build/frontendIdentity.node-check.mjs, neither of which is included in the production bundle. A rebuild after a test-only change therefore receives a new frontendBuild, and workspaceExperimentStore.ts:108 separates its observations into a different group even though users are running the same frontend behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: feaf5a0800
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Comparison exports now use version3 with stable observation IDs and frontend input fingerprints. | ||
| The comparison page imports version2/3 files locally, validates the full batch before accepting it, |
There was a problem hiding this comment.
This changes the documented contract to version-3 files with import support, but docs/product/WORKSPACE_OVERHAUL.md:59 still describes the comparison as session-only and version 2, while its walkthrough at line 85 only tells users to export before reloading. That file presents itself as the complete resource/feature map, so leaving it unchanged gives contributors and users contradictory guidance about the supported format and recovery flow; update the map and walkthrough alongside this new claim.
Useful? React with 👍 / 👎.
…omparisons # Conflicts: # docs/STATUS.md # docs/product/WORKSPACE_OVERHAUL_VALIDATION.md
|
Latest automatic comparison comments are triaged once as MEDIUM follow-through under #2808. The UUID/SubtleCrypto LAN compatibility and resolved Vite configuration findings are implemented in child PR #2856. Remaining comparison improvements are to retain resolved light/night appearance when the selected theme is Auto, exclude test-only inputs from the production build fingerprint, and align the canonical feature-map/export walkthrough with version3 import behavior. These affect descriptive grouping precision/documentation, not account privacy or stored observation retention; no randomized/statistical A/B claim is made. They remain in the comparison continuation queue, without a fix cascade on the already reviewed parent. |
1 participant
Comparison notes can now be carried into a new session or release through validated version2/3 files. Imports retain build attribution, skip exact duplicates, reject conflicting IDs and validate the whole batch before accepting any observations. The results table groups identical scenario, experience, presentation, theme and build conditions, with explicit completed/blocked counts and optional ratings. Notes remain manually retained files; no server telemetry or automatic assignment is added.
Production builds embed a SHA256 fingerprint of frontend inputs and public build configuration, distinct from the backend version. Development and older exports retain unknown frontend attribution. This identifies build inputs, not signed release artifacts. The four selectable experiences and Grove themes remain available.
Validation:
Refs #2808