fix: strip the Italian archdiocesan type word (58 slugs) - #2
Merged
Conversation
The slugify() strip rule used `^(arch)?diocesi di |^(arch)?diocese of `.
Italian forms the archdiocese as "arcidiocesi", not "archdiocesi", so the
optional `arch` prefix never matched the Italian styled form and all 58
Italian archdioceses kept the type word in their slug, contrary to rule 1
of the schema proposal ("the type is an attribute, not part of the
identity").
Split the alternation so each language keeps its own form and regenerate
the seed. 58 IDs change, e.g. circ:it-arcidiocesi-di-acerenza ->
circ:it-acerenza. No collisions: 2,935 entries, 2,935 unique IDs.
Sweeping the regenerated seed for surviving type words leaves 8 Italian
territorial prelatures and abbacies (circ:it-abbazia-territoriale-di-
montecassino, circ:it-prelatura-territoriale-di-loreto). Whether those
reduce to the bare see name is a separate naming question for the
committee and is left unchanged here.
All IDs remain drafts pending committee review (#1).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe seed generator now handles Italian and English diocese prefixes separately. Italian archdiocese identifiers in the circumscriptions registry now omit the ChangesItalian Circumscription Identifier Normalization
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
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.
Fixes the strip rule for Italian archdioceses. The cohort was surfaced by @damienriehl in #1; the diagnosis here is different from the one recorded there, and this PR fixes the cause.
The defect
slugify()stripped generic type prefixes with a single alternation:Italian forms the archdiocese as arcidiocesi, not archdiocesi. The optional
archprefix therefore never matched the Italian styled form, and all 58 Italian archdioceses kept the type word in their slug — contrary to rule 1 of the schema proposal, "the type is an attribute, not part of the identity".The source index uses exactly four styled forms (
Diocesi di×149,Diocese of×143,Arcidiocesi di×58,Archdiocese of×32); three of the four were handled. Splitting the alternation so each language keeps its own form covers all four:Effect
58 IDs change. No collisions — 2,935 entries, 2,935 unique IDs (the generator's duplicate assertion passes).
Sweeping the regenerated seed for surviving type words now returns nothing in the diocesan/archdiocesan cohort.
Deliberately out of scope
Two cohorts still carry a type word, both by design rather than by defect:
circ:de-deutsches-militarordinariat,circ:pl-ordynariat-polowy-wojska-polskiego). Rule 5 already provides for these and names one in exactly this form. A military ordinariate has no see — its territory is a body of armed forces, not a place — so there is no place name beneath the type word to reduce to. Stripping it leavescirc:de-.circ:it-abbazia-territoriale-di-montecassino,circ:it-prelatura-territoriale-di-loreto). Whether these should reduce tocirc:it-montecassino/circ:it-loretounder rule 1 is a genuine naming question, but it is a committee decision about eight entries rather than a generator bug, and it is left untouched here.Status
All IDs remain drafts pending committee review, as the seed's own
$commentrecords. This changes 58 of them while the registry is still pre-publication, which is when identifiers are revisable.🤖 Generated with Claude Code
Summary by CodeRabbit