TypeSchema/TS: emit validateRequired() for inherited base-required fields#166
Merged
ryukzak merged 7 commits intoJun 1, 2026
Merged
Conversation
Profile snapshots silently dropped any base-resource cardinality the profile differential did not re-state. Downstream consumers (generated validate() methods, schema diff tooling) had no way to know that, e.g., a Provenance profile still requires `target` and `recorded` from R4 core. This commit fixes the snapshot itself; the TS writer change lands in a separate commit so each layer can be reviewed independently. - types.ts: SnapshotProfileTypeSchema gains an optional `inheritedRequiredFields: string[]` listing top-level fields the base resource marks required and the profile chain leaves untouched. - utils.ts (buildProfileSnapshot): walks the base resource's `fields` after flatProfile() and collects every entry with `required: true` whose name is absent from the merged differential fields. Listed separately from `fields` so this fix only enables additional validation — it does not inflate the profile's getter/setter surface with unused base metadata (e.g. value[x] variants the profile bans). - utils.ts (flatProfile merge loop): when a profile in the chain sets `min: 0` on a field, the merged field's `required` is reset to false. Required for the relaxation case so a profile that intentionally drops a base requirement doesn't accidentally still trigger validateRequired() emission.
Generated profile validate() methods only emitted validateRequired()
for fields the profile differential mentioned. Inherited cardinality
from the base R4 resource (e.g. Provenance.target, .recorded) was
silently skipped, so a downstream resource could pass .validate() in
TypeScript and then be rejected by the FHIR server for missing the
base requirement.
After the snapshot now carries `inheritedRequiredFields`, the writer
just appends one validateRequired() per entry at the end of the errors
array — the regular field loop still handles fields the profile
restates (so AC-3 — profile relaxes with min:0 — falls out naturally
because the field stays in `fields` with required:false).
Before:
return {
errors: [
...validateRequired(res, profileName, "agent"),
...validateRequired(res, profileName, "activity"),
],
warnings: [],
}
After (Provenance profile that leaves target/recorded untouched):
return {
errors: [
...validateRequired(res, profileName, "agent"),
...validateRequired(res, profileName, "activity"),
...validateRequired(res, profileName, "target"),
...validateRequired(res, profileName, "recorded"),
],
warnings: [],
}
…ation - AC-2: profile leaving base-required fields untouched lists them in inheritedRequiredFields and keeps explicitly-restated fields out of it (mirrors the Provenance.target/.recorded/.agent pattern). - AC-3: profile lowering min to 0 on a base-required field marks the merged field required:false and excludes it from inheritedRequiredFields, so validate() emits nothing for it. - inheritedRequiredFields is undefined when the profile restates every base requirement (no spurious empty arrays).
ExampleTypedBundle profile slices entry[] but does not re-state Bundle.type in its differential. With the inherited-required fix in place, validate() now correctly emits validateRequired() for type.
sussdorff
added a commit
to cognovis/codegen
that referenced
this pull request
May 24, 2026
Replants cognovis/mira-adapters onto cognovis/next + committed dist/. Includes: - Consolidated canonical-resolver fix (codegen-vrq + codegen-pkt) - Inherited-required validate() emission (codegen-8iw, upstream PR atomic-ehr#166) - Preserve fixed CodeableConcept codings (codegen-vwd, upstream PR atomic-ehr#165) - COGNOVIS strategy doc + bd init infrastructure - KBV newer-deps repro script
The first cut filtered base-required fields out as soon as the profile mentioned them — but profile snapshots frequently inline inherited elements without restating the required flag (FHIRSchema copies the snapshot expansion verbatim). The field-builder then emits target/recorded with required:false even though the base requires them, and the regular validate loop skipped them too. Re-derive: a base-required field is "inherited" unless the profile either restated required:true (already covered by the regular loop) or explicitly relaxed min to 0. Adds AC-2b to lock the snapshot- inlined case to the same behaviour as a profile that omits the field entirely.
…ission End-to-end coverage via localStructureDefinitions: a Provenance profile whose differential touches only activity/agent/entity must still emit validateRequired() for the inherited base-R4 target and recorded fields. Fixture mirrors the real de.cognovis.fhir.praxis PraxisProposalProvenance differential — the profile that originally surfaced codegen-8iw.
… validated The profile-helpers import guard keyed solely on snapshot.fields being non-empty. A profile whose only validation comes from an inherited base-required field (e.g. an Extension profile relying on the base Extension.url) emitted validateRequired() without importing it → tsc TS2304 "Cannot find name 'validateRequired'". Surfaced by the upstream Test SDKs CI (on-the-fly norge-r4 example, Extension_NoBasisMiddlename). Extend the guard to also fire when inheritedRequiredFields is non-empty, and cover it with a minimal Extension fixture that states no differential fields.
5 tasks
This was referenced Jun 1, 2026
Collaborator
|
Follow-ups now that this is merged:
|
sussdorff
added a commit
to cognovis/codegen
that referenced
this pull request
Jun 2, 2026
…nherited-required choice-skip + atomic-ehr#166 inherited-required validation (f027e0f)
Contributor
Author
|
Thank you! highly appreciated. So far it did not find any issues with the new code generation on our end, so this looks good to us. |
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.
Summary
Generated code (Provenance profile that does NOT re-state target/recorded)
Before:
```typescript
validate(): { errors: string[]; warnings: string[] } {
const profileName = "PraxisProposalProvenance"
return {
errors: [
...validateRequired(res, profileName, "activity"),
...validateRequired(res, profileName, "agent"),
],
warnings: [],
}
}
```
After:
```typescript
validate(): { errors: string[]; warnings: string[] } {
const profileName = "PraxisProposalProvenance"
return {
errors: [
...validateRequired(res, profileName, "activity"),
...validateRequired(res, profileName, "agent"),
...validateRequired(res, profileName, "target"),
...validateRequired(res, profileName, "recorded"),
],
warnings: [],
}
}
```
Tests