chore: fix pre-existing main breakages blocking clean overlay composition (#821) - #832
Draft
aarontrowbridge wants to merge 3 commits into
Draft
chore: fix pre-existing main breakages blocking clean overlay composition (#821)#832aarontrowbridge wants to merge 3 commits into
aarontrowbridge wants to merge 3 commits into
Conversation
… overlay files The committed manifest lagged the overlay by three files added in #774 (packages/session-ui/src/v2/components/{diff-gutter-extension, editable-diff-view-core,editable-diff-view}.ts): present in overlay/, absent from manifest.files — the drift gate fails on pristine main with 'stray files in overlay/ not in the manifest'. Repair path per #796: recompute the manifest from the committed overlay against the v1.18.29 upstream tree (scripts/refresh_manifest.mjs).
… shape Canonical's server-session.ts builds its store literal without diff_version (v1.18.29 and current upstream HEAD alike) and passes that draft to dropSessionCaches — which the overlay types with diff_version REQUIRED, so the materialized tree fails typecheck at server-session.ts(497): error TS2741: Property 'diff_version' is missing ... but required in type 'SessionCache' The field is an overlay-only cache-invalidation counter (no canonical wire shape to match), so the reconciliation is to widen the parameter type: diff_version becomes optional on SessionCache and the delete is guarded. The overlay's own global store type (global-sync/types.ts State) still declares diff_version required — only dropSessionCaches' parameter widens.
…ocales The overlay's i18n parity test fails on pristine main: en.ts carries "session.exportTrace" (added at the fork pin) but none of the 17 non-English app locale dicts do — the parity test aborts on the first locale (ar) with missing: ["session.exportTrace"]. The ui and desktop domains are clean. Issue #821 attributes this to the 'am' locale; that attribution traces to PR #819's review notes and does not reproduce: the overlay carries no am dict and its parity test checks the 17 locales it owns. The reproducible failure is the missing key across all 17, restored here following each table's own terminology (br is pt-BR upstream).
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
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
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.
Closes #821. Fixes the two pre-existing breakages that red every overlay composition check on pristine origin/main, verified on a fresh
origin/main(6afdb9d) worktree and materialized tree before and after.Breakage 1 —
server-session.ts(497)diff_versiontype clashRepro: materialize (canonical v1.18.29 + overlay) →
tsgo -binpackages/app:error TS2741: Property 'diff_version' is missing ... but required in type 'SessionCache'.Root cause: canonical's
server-session.tsbuilds its store literal withoutdiff_version(v1.18.29 and current upstream HEAD) and passes that draft todropSessionCaches, which the overlay types withdiff_versionrequired. The field is an overlay-only cache-invalidation counter — there is no canonical wire shape to match — so the reconciliation widens the parameter type:diff_versionbecomes optional onSessionCacheand the delete is guarded. The overlay's own global store type (global-sync/types.tsState) still declares it required; onlydropSessionCaches' parameter widens.Verification: the materialized typecheck error set on the fixed branch is byte-identical to the base set minus exactly this one error (19 → 18; the remaining 18 are the known overlay-vs-v1.18.29 API drift on surfaces PR #819 re-homes — out of scope here, no drive-bys).
Breakage 2 — i18n parity failure
Repro:
bun test packages/app/src/i18n/parity.test.tsin the materialized tree fails withmissing: ["session.exportTrace"](en.ts has the key — added at the fork pin — none of the 17 non-English app locale dicts do; the ui and desktop domains are clean).On the issue's 'am' attribution: it traces to PR #819's review notes and does not reproduce — the overlay carries no
amdict and its parity test checks the 17 locales it owns; the materialized tree's only parity failure is this missing key across all 17. Restored the key in all 17, following each table's own terminology (bris pt-BR upstream). Inventing overlayamdicts was rejected deliberately: in the composed tree they would overwrite canonical's complete Amharic tables with a smaller fork-era keyset — the opposite of restoring keys.Also (third pre-existing main breakage, surfaced by the repo's own tooling)
refresh_manifest.mjs/drift_gate.mjsrevealed that the committed manifest lags the overlay by the three#774session-ui files (diff-gutter-extension.ts,editable-diff-view-core.ts,editable-diff-view.tsx— inoverlay/, absent frommanifest.files), so the drift gate fails on pristine main with "stray files". Commit 1 adopts them via the canonical repair path (refresh_manifest.mjsagainst the v1.18.29 upstream tree). Flagging it here since it's adjacent to #821's scope but not named by it — happy to split it out if reviewers prefer.Commits (each green, drift gate PASS at each)
SessionCachewith canonical server-session shape (optionaldiff_version)session.exportTracein the 17 non-English app localesVerification summary
579 files, overlay and manifest in sync).server-session.ts(497)error gone; parity test 4/4 pass.global-syncunit tests: 37 pass, unchanged from base (the 2 bun-test Solid-server module errors are identical on pristine main — environment artifacts, not touched).session-cache.ts, i18n tables,manifest.jsononly).