Skip to content

fhir: map primitive elements#1678

Open
hunterachieng wants to merge 5 commits into
mainfrom
feature/1557-fhir-underscores
Open

fhir: map primitive elements#1678
hunterachieng wants to merge 5 commits into
mainfrom
feature/1557-fhir-underscores

Conversation

@hunterachieng
Copy link
Copy Markdown
Contributor

@hunterachieng hunterachieng commented May 26, 2026

Summary

Add ability to map primitive elements in fhir by focusing on elements starting with an _.
Regenerated fhir-eswatini with tests to check behavior

  • Ensured we do not throw away primitive elements in generate-schema then mapped by checking if the parent has an extension.

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to
know!):

  • I have used Claude Code
  • I have used another model
  • I have not used AI

You can read more details in our
Responsible AI Policy

Review Checklist

Before merging, the reviewer should check the following items:

  • Does the PR do what it claims to do?
  • If this is a new adaptor, added the adaptor on marketing website ?
  • If this PR includes breaking changes, do we need to update any jobs in
    production? Is it safe to release?
  • Are there any unit tests?
  • Is there a changeset associated with this PR? Should there be? Note that
    dev only changes don't need a changeset.
  • Have you ticked a box under AI Usage?

Signed-off-by: Hunter Achieng <achienghunter@gmail.com>
Signed-off-by: Hunter Achieng <achienghunter@gmail.com>
Copy link
Copy Markdown
Collaborator

@josephjclark josephjclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a bit of confusion over whether we're handling primitve types or primitive extensions? I'm expecting that primitive types work already, and we're just adding support for extensions here

Comment thread packages/fhir-eswatini/test/resources/Patient.test.ts
Comment thread packages/fhir-eswatini/test/resources/Patient.test.ts Outdated
import type * as FHIR from "../fhir";
type MaybeArray<T> = T | T[];

export type Observation_SzVitalSigns_Props = {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nervous about this diff! This looks unrelated to the work. And I think all the types here are actually broken?

Copy link
Copy Markdown
Contributor Author

@hunterachieng hunterachieng May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josephjclark I used this command to regenerate the schema pnpm generate-fhir fhir-eswatini --respec could this be the cause of the changes? Because I got this error when testing and I had to export it from builders in fhir-4
TypeError: dt.ensureConceptText is not a function

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange 🤔

When I rebuild from the root I also get strange type errors and wierd syntax. When I go into the adaptor folder and do pnpm build:src --respec, I get different output.

The two commands really should be aliases of each other. I fear some path has gotten misaligned and is doing wierd things.

if you go to main and run pnpm build:src --respec from inside the adaptor folder, what happens?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2026-05-27 at 16 21 13

@josephjclark It is only this that is updated "adaptorGeneratedDate": "2026-05-27T13:18:19.262Z",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I get too.

So I think we have a bug when generating from the repo root. Or maybe it's a bug when generating inside the folder. Either way that needs investigating.

Maybe you can run pnpm build:src --respec on this branch and commit the changes - that should resolve some of the issues I see on this PR.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean the primitive extension no longer gets added? The test fails?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not added, and the tests fail. I had to skip them

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't skip them - if tests are failing its because something is broken. They should fail in CI until we fix the problem.

Something seems very messed up here. I'll try and take a look tomorrow but if I can't, it'll be Monday before I can investigate.

npm generate-fhir fhir-eswatini --respec and pnpm build:src --respec should be exactly the same command. I'm really nervous that we seem to be getting different behaviours. Can you try and investigate what might be different when running the commands?

There's no magic here, just code. All the answers are in this repo .

I suspect something is wrong on main, and we need to update fhir-eswatini on a branch off main first, and then rebase your work here onto that branch. That would help me understand what's wrong on main, and what you've added on this PR. That will make my review a lot easier.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. On it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josephjclark using npm generate-fhir fhir-eswatini --respec does not work. It flags that npm error Missing script: "generate-fhir"
When I try with pnpm when inside fhir-eswatini, I get these changes
Screenshot 2026-05-29 at 12 17 42

declare const setValues: (url: any, values: any, type?: string) => void;
declare const value: (value: any, system: any, ...extra: any[]) => any;

import "./values";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite nervous about this cahnge too

Comment thread tools/generate-fhir/src/generate-schema.ts Outdated
Comment thread tools/generate-fhir/test/generate-code.test.ts Outdated
Signed-off-by: Hunter Achieng <achienghunter@gmail.com>
Signed-off-by: Hunter Achieng <achienghunter@gmail.com>
Signed-off-by: Hunter Achieng <achienghunter@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants