Skip to content

fix(schemas): accept shared translations for new diagram types - #758

Open
outlier27-cell wants to merge 9 commits into
tt-a1i:devfrom
outlier27-cell:fix/new-schema-translations
Open

outlier27-cell wants to merge 9 commits into
tt-a1i:devfrom
outlier27-cell:fix/new-schema-translations

Conversation

@outlier27-cell

@outlier27-cell outlier27-cell commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Problem and value

Fixes #757.

Final CI also exposed an existing browser-fixture isolation failure: Chrome file-backed motion storage was not reliably cleared between new fixtures. The follow-up removes only the test motion preference after ordinary fixture navigation and reloads before measuring startup. Storage-failure fixtures remain intact, and preserved-storage reload assertions still verify the real Viewer behavior. Real Chrome Motion Governor tests pass 8/8 without skips; no production Viewer code changes.

Tree, Class, Timeline, and Waterfall reject the documented meta.translations field before shared Viewer localization can use it. Add the existing shared schema reference to these four metadata objects and regenerate portable validators, following the earlier ERD compatibility fix #684.

Stability impact

Shared schema compatibility: permits the already implemented optional per-key override for these four types. Invalid non-string values remain rejected; absent and empty overrides preserve artifact bytes. No new API, locale router, or translation rules.

Tests run

  • Base dev f4c8f3d7; each affected renderer rejects valid overrides before the fix.
  • node --test test/new-type-translations.test.mjs test/generate-validators.test.mjs: 9/9 passed. Public CLI coverage checks embedded overrides, English fallback, empty/absent byte equivalence, and invalid values in all four types, plus portable validator consistency.
  • Final-head remote CI required. No browser or perceptual acceptance is claimed; this uses the existing Viewer message mechanism without changing layout.

Generated artifacts

Portable generated-validators.mjs regenerated with the maintained generator. archify.zip rebuilt using official Node 22. Example and Viewer inputs are unchanged.

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⛔ Files ignored due to path filters (1)
  • archify.zip is excluded by !**/*.zip
⚙️ Run configuration
  • Configuration used: Repository: tt-a1i/archify/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f6f774b9-9499-4312-85fd-b032adbe49f7
📥 Commits

Reviewing files that changed from the base of the PR and between 9a25cfa and c48d572.

⛔ Files ignored due to path filters (1)
  • archify.zip is excluded by !**/*.zip

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: tt-a1i/archify/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3c074581-d4b8-4165-ac5f-12fb90aed5ab
📥 Commits

Reviewing files that changed from the base of the PR and between 91711da and 9a25cfa.

📒 Files selected for processing (1)
  • test/motion-governor-browser.test.mjs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Summary

Reviewed base f4c8f3d7f13b96643f6bb0736cd9f6575f2b1102 and head 9a25cfa6e810ae93abd0212682cbca11ea82026d. Tree, Class, Timeline, and Waterfall now accept meta.translations through the shared schema, allowing valid per-key overrides while retaining string validation and existing fallback behavior. The author reports focused tests passed 9/9. The ERD precedent in #684 is historical evidence from its original revision, not validation of this change. No CI result was observed for the checked head. Browser or perceptual acceptance is not established.

Walkthrough

The class, timeline, tree, and waterfall schemas now accept optional meta.translations using the shared translations definition. CLI tests check custom translations, English fallback, empty versus omitted translations, and rejection of numeric translation values. A browser test resets the stored motion preference after fresh navigation when storage is not preserved and no fixture is used.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 9a25c

The four diagram types can accept optional shared translation overrides while retaining the existing validation rules. The inspected generated artifacts and browser-test flow reveal no merge-blocking issue.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Contribution Scope ⚠️ Warning The schema change has a focused scope, a linked issue (#757), and adequate compatibility and failure-behavior notes. The PR also changes test/motion-governor-browser.test.mjs in three independent co… Remove the unrelated test/motion-governor-browser.test.mjs changes and their commits, or move them to a separate PR. If maintainers intend to keep them in this PR, record an explicit scope decision and add a concise rationale that explain…
✅ Passed checks (1 passed)
Check name Status Explanation
Validation Evidence ✅ Passed PASS. Evaluated base 70a6dfa and head 9a25cfa. The source diff adds the shared translations reference to all four affected metadata sc…
Full details: Contribution Scope

Explanation

The schema change has a focused scope, a linked issue (#757), and adequate compatibility and failure-behavior notes. The PR also changes test/motion-governor-browser.test.mjs in three independent commits. That test-only fixture-isolation change is unrelated to shared translations, and the authored description gives no rationale or maintainer scope decision for it. This conflicts with CONTRIBUTING.md's focused-change rule and the PR template's requirement to explain changed behavior and callers.

Resolution

Remove the unrelated test/motion-governor-browser.test.mjs changes and their commits, or move them to a separate PR. If maintainers intend to keep them in this PR, record an explicit scope decision and add a concise rationale that explains their relation to this delivery slice.

  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant