Skip to content

fix(register): resolve cross-package + transitive-version + cache-mismatch canonical lookups#151

Open
sussdorff wants to merge 2 commits intoatomic-ehr:mainfrom
cognovis:fix/canonical-resolver-cache-mismatch
Open

fix(register): resolve cross-package + transitive-version + cache-mismatch canonical lookups#151
sussdorff wants to merge 2 commits intoatomic-ehr:mainfrom
cognovis:fix/canonical-resolver-cache-mismatch

Conversation

@sussdorff
Copy link
Copy Markdown
Contributor

Summary

Three related canonical-resolver bugs caused 'Base resource not found' errors when packages have different versions of the same dependency. Discovered while running codegen on @polaris (fhir-de) which loads de.basisprofil.r4@1.6.0-ballot2 (top-level) plus kbv.basis@1.8.0 (transitive, depends on de.basisprofil.r4@1.5.4) plus FHIR IG packages via localTgzPackage().

This PR consolidates three iterative fixes from our cognovis fork (vrq → 7y9 → pkt) into a single change.

Bugs Fixed

1. Null-id .index.json rejection causes 0 indexed resources

de.basisprofil.r4@1.5.4 ships an .index.json with null-id ImplementationGuide entries. parseIndex treats any null id as fatal and rejects the entire .index.json — leaving the package with 0 indexed resources. Cross-package base-type lookups for KBV profiles (e.g. KBV_PR_Base_Observation_Care_Levelobservation-de-pflegegrad) fail at transform time.

Fix: When manager.search returns 0 for a focused package, fall back to a direct directory scan of the canonical-manager's node_modules cache (scanNodeModulesPackage). Equivalent to the manager's own scanDirectoryForResources but tolerant of malformed .index.json.

2. Top-level + transitive package version mismatch

When the top level loads de.basisprofil.r4@1.6.0-ballot2 and a transitive dep (kbv.basis) needs de.basisprofil.r4@1.5.4, npm/bun installs the older version into a nested location: node_modules/kbv.basis/node_modules/de.basisprofil.r4/. The original scanNodeModulesPackage only checked the flat top-level path → returned wrong-version content silently.

Fix: scanNodeModulesPackage now reads package.json version from the flat path; if it doesn't match the requested version, scans nested node_modules/<parent>/node_modules/<dep>/ paths. Returns the first one whose version matches; falls back to flat path with a warning if no exact match.

3. APIBuilder.localTgzPackage cache-key divergence

When packages are added via APIBuilder.localTgzPackage() (or manager.addTgzPackage()), the canonical-manager's cache hash is computed from constructor packages only. addTgzPackage adds packages to the cache record but doesn't change the hash. Codegen however computed nodeModulesPath from ALL focusedPackages (those returned by manager.init(), including tgz adds) → divergent hashes → computed path doesn't exist → fallback never triggers → silent 0-resource failure.

Concrete failure: a build like
```ts
new APIBuilder()
.fromPackage('hl7.fhir.r4.core', '4.0.1')
.fromPackage('de.basisprofil.r4', '1.6.0-ballot2')
.localTgzPackage('vendor/de.cognovis.fhir.dental-0.17.0.tgz')
.typescript({})
.outputTo('generated/dental')
```
returns Base resource not found 'http://fhir.de/StructureDefinition/observation-de-pflegegrad' despite the resource existing in the actual cache.

Fix: When the computed nodeModulesPath does not exist, scan ALL sibling cache record directories under <workdir>/canonical-manager-cache/ and try each one. The warning message identifies which cache record was actually scanned.

Test plan

10 tests across 2 files, all passing:

  • test/unit/typeschema/versioned-canonical.test.ts (5 tests):
    • cross-package base type lookup with kbv.basis@1.8.0 + de.basisprofil.r4@1.5.4
    • covers the null-id .index.json scenario via real package downloads
  • test/unit/typeschema/transitive-version-mismatch.test.ts (5 tests):
    • Integration: 1.6.0-ballot2 top-level + kbv.basis@1.8.0 resolves without errors
    • Synthetic: scanNodeModulesPackage prefers nested 1.5.4 over flat 1.6.0-ballot2
    • Synthetic: scanNodeModulesPackage scans sibling cache dirs when computed path missing

Run with:
```bash
bun test test/unit/typeschema/versioned-canonical.test.ts test/unit/typeschema/transitive-version-mismatch.test.ts
```

Background

This branch consolidates three iterative fixes from our cognovis/mira-adapters fork:

  • codegen-vrq: introduced nodeModulesPath fallback (Bug 1)
  • codegen-7y9: added nested-path version-aware scan (Bug 2)
  • codegen-pkt: added sibling-cache scan for localTgzPackage layouts (Bug 3)

The single squashed commit makes review simpler. Happy to split into per-bug commits if preferred.

We've been running this on production codegen for polaris — verified 11/11 builders complete cleanly.

…match canonical lookups

The canonical resolver had three related bugs causing 'Base resource not found' errors
when packages have different versions of the same dependency. This commit consolidates
three iterations (vrq, 7y9, pkt) of the fix into a single change.

## Bug 1: kbv.basis@1.8.0 .index.json has null id entries

de.basisprofil.r4@1.5.4 ships an .index.json with null-id ImplementationGuide entries.
parseIndex treats any null id as fatal and rejects the entire .index.json — leaving
the package with 0 indexed resources. Cross-package base-type lookups for KBV profiles
that inherit from de.basisprofil.r4 fail at transform time.

Fix (vrq): when manager.search returns 0 for a package, scan the canonical-manager's
node_modules cache directly. Equivalent to scanDirectoryForResources but tolerant of
malformed .index.json. Implemented as scanNodeModulesPackage with a nodeModulesPath
parameter computed from the focusedPackages cache key.

## Bug 2: top-level + transitive package version mismatch

When the top level loads de.basisprofil.r4@1.6.0-ballot2 and a dependency (kbv.basis)
needs de.basisprofil.r4@1.5.4, npm installs the older version into a nested location:
node_modules/kbv.basis/node_modules/de.basisprofil.r4/. The original scanNodeModulesPackage
only checked the flat top-level path → returned wrong-version content.

Fix (7y9): scanNodeModulesPackage now reads the package.json version from the flat path,
and if it doesn't match the requested version, scans nested
node_modules/<parent>/node_modules/<dep>/ paths. Returns the first one whose version
matches; falls back to flat path if no match (graceful degradation).

## Bug 3: APIBuilder.localTgzPackage cache-key divergence

When packages are added via APIBuilder.localTgzPackage() (or addTgzPackage on the
manager), the canonical-manager's cache hash is computed from constructor packages
ONLY. addTgzPackage adds packages to the cache record but doesn't change the hash.
Codegen however computed nodeModulesPath from ALL focusedPackages → divergent hashes
→ computed path doesn't exist → fallback never triggers → silent failure.

Concrete failure: polaris dental builder uses .localTgzPackage(praxis) +
.localTgzPackage(dental). manager.search returned 0 for de.basisprofil.r4@1.6.0-ballot2
even though the resource existed in the actual cache directory.

Fix (pkt): when the computed nodeModulesPath does not exist, scan ALL sibling cache
record directories under <workdir>/canonical-manager-cache/ and try each one. The
warning message identifies which cache record was actually scanned.

## Tests

- test/unit/typeschema/versioned-canonical.test.ts (5 tests): cross-package base type
  lookup with kbv.basis@1.8.0 + de.basisprofil.r4@1.5.4 against null-id .index.json scenario
- test/unit/typeschema/transitive-version-mismatch.test.ts (5 tests):
  - Integration: 1.6.0-ballot2 top-level + kbv.basis@1.8.0 → no errors
  - Synthetic: scanNodeModulesPackage prefers nested 1.5.4 over flat 1.6.0-ballot2
  - Synthetic: scanNodeModulesPackage scans sibling cache dirs when computed path missing

All 10 tests pass on upstream/main.
@sussdorff
Copy link
Copy Markdown
Contributor Author

Thanks for pointing to the kbv-r4 example and the patch-on-load pattern.

I tested all three of the failure cases this PR addresses against preprocessPackage + ignorePackageIndex: true with the resolver fallback bypassed (canonical-manager 0.0.23, driven via manager.search directly, no register-layer fallback). Result:

  1. Null-id .index.json in de.basisprofil.r4@1.5.4 — fully solved by ignorePackageIndex: true alone. Without the hook, manager.search returns 0 resources for the package; with it, all 168 are indexed and observation-de-pflegegrad|1.5.4 resolves cleanly. preprocessPackage isn't even needed for this case (we don't hit the missing-core-dep variant the kbv-r4 example fixes).

  2. APIBuilder.localTgzPackage cache-key divergence — turns out to have been a downstream symptom of (1). The original failure was manager.search returning 0 because of the broken index; our fallback then ran with a divergent path hash and silently found nothing. With ignorePackageIndex: true the manager indexes packages directly, the fallback isn't taken at all, and the hash divergence is moot. Effectively solved as a side-effect.

  3. Nested transitive version layout (top-level de.basisprofil.r4@1.6.0-ballot2, kbv.basis/node_modules/de.basisprofil.r4@1.5.4) — this one isn't reachable by either hook. Reading scanner/directory.js, loadPackagesIntoCache only iterates top-level node_modules entries; the nested 1.5.4 is invisible regardless of ignorePackageIndex. This case only manifests in npm-style installs — bun flattens to a single version, so it's a non-issue in your usual environment. We hit it because Polaris's CI runs on npm, where the older transitive version genuinely lives in a nested path.

One related caution from our side: in Polaris we previously had a preprocessPackage workaround in place to inject a missing dependency, and it ended up masking a real resolver bug for several months — every regeneration just silently picked up wrong-version content, and removing the workaround is what surfaced the actual root cause. So I'm a bit wary of patch-on-load as the only layer for these defects, especially when the patch can hide divergent behavior rather than just normalize metadata. That said, ignorePackageIndex: true is qualitatively different — it disables a broken cache rather than mutating metadata, so it doesn't carry the same masking risk.

Happy to drop the resolver code paths that are covered by ignorePackageIndex: true (which is most of this PR) and keep only the nested-transitive handling, gated on a clear opt-in. Or, if you'd prefer a different shape — for instance handling nested layouts inside the canonical-manager scanner itself — I'm fine to close this PR and move that discussion there. What's the preferred direction?

@ryukzak
Copy link
Copy Markdown
Collaborator

ryukzak commented Apr 30, 2026

Currently, I will investigate a problem deeply to find a good way how it should work with conflict versions and also how to leave a control on the user’s side. I will return here soon. Looks like it should be a different approach for regular and core packages.

ryukzak added a commit that referenced this pull request Apr 30, 2026
Replace the explicit hl7.fhir.r4.core dep (now pulled transitively)
with de.basisprofil.r4@1.6.0-ballot2 and kbv.basis@1.8.0 to
exercise the cross-package version graph from PR #151.
@ryukzak
Copy link
Copy Markdown
Collaborator

ryukzak commented Apr 30, 2026

Hi @sussdorff!

Did I understand your setup correctly: you want types for kbv.ita.for@1.3.1 but with newer transitive versions:

  • de.basisprofil.r4@1.5.2de.basisprofil.r4@1.6.0-ballot2
  • kbv.basis@1.7.0kbv.basis@1.8.0

specified like this:

.fromPackage("kbv.ita.for", "1.3.1")
.fromPackage("de.basisprofil.r4", "1.6.0-ballot2")
.fromPackage("kbv.basis", "1.8.0")

and you hit:

X api: Code generation failed: Could not resolve field type:
<http://hl7.org/fhir/StructureDefinition/biologicallyderivedproduct-manipulation>.valueCodeableReference:
<CodeableReference> (pkg: 'hl7.fhir.uv.extensions.r4#5.2.0')

If so — the failure is in hl7.fhir.uv.extensions.r4@5.2.0, a new transitive dep introduced by de.basisprofil.r4@1.6.0-ballot2. That package is broken: it ships R4-targeted extensions whose value[x] choices are typed as CodeableReference (and Availability) — types that don't exist in R4. So codegen can't resolve them.

For cases like this we already maintain a hardcoded skip list in src/typeschema/skip-hack.ts — same package, six entries already. Two more were missing (biologicallyderivedproduct-manipulation, biologicallyderivedproduct-processing); I've added them. The "hack" naming is intentional — long-term we want to detect R5-only types automatically rather than maintain the URL allowlist by hand, but this gets you unblocked today.

I also improved the error message for this case so the next person hits a clearer signal:

Could not resolve field type:
  package: hl7.fhir.uv.extensions.r4#5.2.0
  schema:  http://hl7.org/fhir/StructureDefinition/biologicallyderivedproduct-manipulation
  field:   valueCodeableReference
  type:    CodeableReference
  hint:    'CodeableReference' is an R5+ type and is not available when generating against R4.
           Either skip this canonical via skip-hack.ts, or upgrade the target to R5.

All three changes are in #153 — please give it a look and let me know if it covers your case.

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.

2 participants