Skip to content

fix(scaffold): discover the Ambrosian sanctorale per edition (#101) - #103

Merged
JohnRDOrazio merged 1 commit into
mainfrom
fix/101-ambrosian-sanctorale-per-edition
Oct 10, 2026
Merged

JohnRDOrazio merged 1 commit into
mainfrom
fix/101-ambrosian-sanctorale-per-edition

Conversation

@JohnRDOrazio

Copy link
Copy Markdown
Member

Closes #101

Problem

API#963 (merged 2026-09-02) added the missal edition to the Ambrosian sanctorale inventory ids:

before after
sanctorale:ambrosian sanctorale:ambrosian:EDITIO_TYPICA_2024
sanctorale:ambrosian:i18n sanctorale:ambrosian:EDITIO_TYPICA_2024:i18n

inventoryIdsForCalendar() still composed the edition-less pair. The API no longer advertises it, so buildSourceDataChecks() logged a warning and skipped both. Every Ambrosian scaffold had silently stopped checking the Ambrosian sanctorale. Only the E2E run caught it (rite-selection.spec.ts:182), on #102, the first run of the workflow since 2026-08-29.

Fix

Nothing in /calendars names the Ambrosian editions, so the non-Roman sanctorale is now discovered from the /validations inventory by prefix (sanctorale:{rite}:), the same way the rite's lectionary sections already are. This follows the rule the code documents: compose what calendar metadata implies, and discover from the inventory what it does not. Hardcoding the edition would break again when 1976 gains data (API#957).

The Roman sanctorale:roman:* family stays composed from national metadata. Discovering it would put every nation's missals on every Roman scaffold.

CONDITIONAL_INVENTORY_ID is unchanged. Comments that cited sanctorale:ambrosian:i18n as a live id now explain that it is the pre-#963 shape.

Tests

  • ws-protocol.spec.ts: the composed Ambrosian expectations (rite-level and diocesan) drop the sanctorale pair.
  • ws-protocol.spec.ts: a new spec checks every composed Ambrosian rite-level id against the live /validations, and that at least one sanctorale:ambrosian:{edition} is there to be discovered. The existing "every composed id is advertised" spec only covered a Roman scope, which is why this drift went unnoticed.
  • rite-selection.spec.ts: pins the Ambrosian edition/-i18n card pairing, in the same way as the Roman missals, so a new edition needs no edit.
  • resources-rite.spec.ts and the wsProtocol.js docblock: id spelling corrected to EDITIO_TYPICA_2024.

Issue #101 also suggested deriving the expected-id arrays from /validations. Those arrays are the expected output of a pure function, so they stay literal. The live-inventory comparison is in the new spec instead.

Verification

  • npm run typecheck and eslint on the changed JS pass.
  • Against a local API on development, inventoryIdsForCalendar() composes only advertised ids for the Ambrosian rite-level scope. The discovery filter finds sanctorale:ambrosian:EDITIO_TYPICA_2024 and its :i18n, and finds no sanctorale:roman:* under a Roman scope.
  • I could not run the full Playwright suite locally because of a port clash on this machine, so this PR's CI E2E run is the end-to-end check.

Once this is merged, #102 (PIE) can be rebased onto it and should go green.

🤖 Generated with Claude Code

API#963 gave the Ambrosian sanctorale inventory ids a missal-edition
segment (`sanctorale:ambrosian:EDITIO_TYPICA_2024` and its `:i18n`), but
inventoryIdsForCalendar() still composed the edition-less pair. The API no
longer advertised it, so buildSourceDataChecks() warned and skipped both,
and every Ambrosian scaffold silently stopped checking its sanctorale.

Nothing in /calendars names the editions, so the non-Roman sanctorale is
now discovered from the /validations inventory by prefix, the same way the
rite's lectionary sections already are; composing it would mean
hardcoding an edition list that grows as editions gain data (API#957).
The Roman sanctorale:roman:* family stays composed from national metadata.

Tests: the composed Ambrosian expectations drop the sanctorale pair;
rite-selection pins the edition/i18n pairing like the Roman missals; a new
spec checks every composed Ambrosian rite-level id against the live
inventory and that at least one edition is there to be discovered.

Closes #101

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 10, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 34 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d3afa9c9-a06a-42fd-8303-4a9917fc41c1

📥 Commits

Reviewing files that changed from the base of the PR and between b063f7d and b56972a.


📒 Files selected for processing (5)
  • assets/js/index.js
  • assets/js/wsProtocol.js
  • e2e/resources-rite.spec.ts
  • e2e/rite-selection.spec.ts
  • e2e/ws-protocol.spec.ts

  • Autofix · 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.

@JohnRDOrazio
JohnRDOrazio merged commit f96432c into main Oct 10, 2026
5 checks passed
@JohnRDOrazio
JohnRDOrazio deleted the fix/101-ambrosian-sanctorale-per-edition branch October 10, 2026 18:45
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.

Ambrosian sanctorale inventory ids gain their edition — e2e expectations need updating

1 participant