docs: clarify that knownValues is an open vocabulary - #213
Conversation
Adds a 'Schema Conventions' section to README.md explaining that knownValues fields are open: custom string values are wire-valid, the listed values are interop conventions, and only 'enum' fields are closed and validator-enforced. Also documents the strongRef + CID pinning semantic so consumers don't reach for embedded-record shortcuts. Updates inline descriptions on app.certified.location#locationType, org.hypercerts.workscope.tag#category, and org.hypercerts.workscope.tag#status to say 'Values beyond the known set are permitted' — bringing them in line with the existing wording on collection#type, attachment#contentType, and badge.definition#badgeType. The locationType description now also explicitly tells implementers to use the catch-all 'geojson' entry for any non-Point GeoJSON (Polygon, MultiPolygon, FeatureCollection) rather than inventing typed variants. Splits a misleading bullet in STRING_CONSTRAINTS.md that conflated knownValues with enum — the two have different validation semantics (enum is closed, knownValues is open), and knownValues fields still want maxLength to bound the size of custom values. Regenerates SCHEMAS.md. Documentation-only — no schema or type changes.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
🦋 Changeset detectedLatest commit: 80e5b42 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis patch documentation-only update clarifies that ChangesknownValues Open Vocabulary Clarification
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
aspiers
left a comment
aspiers
left a comment
There was a problem hiding this comment.
Fantastic clarification, thanks!
2 participants
Summary
Documents one of the lexicon's quieter conventions:
knownValuesis an open vocabulary, not a closed enum. Lexicon validators do not reject values outside the list, and several lexicons in this repo already document that explicitly (collection#type,attachment#contentType,badge.definition#badgeTypeall say "Values beyond the known set are permitted"), but the rest don't — and there's no top-level explanation of the convention anywhere. This makes it easy for implementers to read aknownValueslist as a closed registry, either rejecting otherwise-valid records on the producer side or treating a custom value as a bug on the consumer side.This PR is documentation-only — no schema or type changes.
Changes
README.md— new "Schema Conventions" sectionAdded after the Lexicon Reference table. Two subsections:
knownValuesis an open vocabulary, not a closed enum. Spells out that custom values are wire-valid, contrasts withenum(which the validator does enforce), and gives practical guidance: prefer a known entry when one matches, custom is fine when none do, but consumers that bucket by the canonical list will treat custom values as "other". UseslocationType: "geojson-polygon"as a concrete example (valid record, but a Location Protocol registry consumer won't recognize it — usegeojsonfor canonical bucketing).strongRefpins to a specific record version. Brief note that the CID inside a strongRef provides snapshot-at-time-of-reference semantics. This is here because the most common mistake people make when they don't internalize this is to embed full records by value to 'snapshot' them — strongRef + CID already does that, and several fields in this repo are typed that way for exactly that reason (badge.award.badge,badge.response.badgeAward,funding.receipt.for).Inline lexicon descriptions
Brought three
knownValuesfield descriptions in line with the existing pattern oncollection#type/attachment#contentType/badge.definition#badgeType:app.certified.location#locationType— adds the open-vocabulary disclaimer, AND explicitly directs implementers to use the catch-allgeojsonentry for non-Point GeoJSON (Polygon, MultiPolygon, FeatureCollection) rather than inventing typed variants likegeojson-polygon. The inner GeoJSON payload's owntypefield carries the specifics.org.hypercerts.workscope.tag#category— adds the disclaimer.org.hypercerts.workscope.tag#status— adds the disclaimer.STRING_CONSTRAINTS.md— split a misleading bulletThe "When to use neither" section had a single bullet conflating
knownValuesandenum:That's correct for
enum(closed, validator-enforced, allowed values are fully defined) but wrong forknownValues(open, custom values still needmaxLengthto bound size). Split into two bullets making the distinction explicit.SCHEMAS.mdAuto-regenerated via
npm run gen-schemas-md. Only the rows whose JSON descriptions changed are affected.What I intentionally did not change
A few
knownValuesfields whose descriptions could also get the disclaimer for consistency, but where the semantics are tighter and adding the disclaimer might mislead implementers in the other direction:badge.response#response(accepted/rejected) — extending with custom variants likeneutralwould change the semantics of the response, and the recipient-side UI only models accept/reject.actor.organization#visibility(public/unlisted) — discoverability semantics; custom variants here likely won't be honored by any platform that reads this field.workscope.cel#version(v1) — schema version; extending it means a new CEL context format and that's a coordinated upgrade, not an at-your-own-risk extension.Happy to add them anyway if reviewers prefer pure consistency over the semantic-tightening signal.
Test plan
npm run gen-schemas-md✅ (SCHEMAS.md regenerated cleanly)npm run format✅ (no formatting drift)npm run check✅ (135 tests pass, including all 92 doc-snippet validation tests — confirms README + SKILL.md code blocks still validate against the lexicon)Changeset
Patch-level — included in this PR.
Summary by CodeRabbit
Documentation