Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fhir v4 #909

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

fhir v4 #909

wants to merge 12 commits into from

Conversation

josephjclark
Copy link
Collaborator

@josephjclark josephjclark commented Jan 14, 2025

WIP PR for fhir 4 adaptor support

  • Download the fhir 4 spec and generate a basic adaptor
  • Implement a full suite of datatype builders, with tests
  • Allow a custom init function to handle metadata/text fields. These are going to be different on each IG so we need a general hook.
  • Use valuesets to allow simple codes to be mapped to full concepts with systems
  • Generate tests with examples?
  • Generate fhir verbs API? read search etc? A bundle builder will be great
  • Implement full and proper extension support? Can I even do this with the base?

AI Usage

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

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our Responsible AI Policy

Core fhir definitions seem use a different bundle structure to the regular IG definitions. Basically the folder structure is a bit different and the resources are all captured in a single bundle.

This commit adds a solution which seems to work in fhir 4 at least
@josephjclark
Copy link
Collaborator Author

Bit of a false start this afternoon as it turns out that the core fhir definitions seem to use a different structure to the definitions downloaded from IG guides. But I figured that out and I've now managed to download the fhir 4 spec and generate a basic adaptor for it. I'll tune that up an get the basics submitted, then start working on improvements.

@josephjclark
Copy link
Collaborator Author

A good dev day today with improvements, plus some good news and mixed.

In brief I have:

  • Restructured the output so that each profile has its own file. It's now possible to read the code, hurrah!
  • Simplified (and complicated) builder signatures so that the profile is optional. You can now do b.patient({ id: 'joe' }) rather than b.patient('patient', { id: 'joe' }) . This needs some work to be properly compatible with IG guides with multiple profiles (we should default to the base profile)
  • Added jsdoc annotations for each profile, so the builders are documented. Yay! This won't work on IGs with multiple profiles.
  • Got code assist working in VSC on unit tests. This has been a bit of pain and does require testing against the built code.

There does seem to be an issue where once we've built to dist, the code assist in the test file breaks down. I need to investigate.

I seriously flirted with adopting a typescript-first implementation today. There are many benefits to this. The biggest reason NOT to is that I have to re-write most of the code generator to use the TypeScript AST (which has a horrible interface)

Also worth noting is one failing test which requires investigation.

Next steps, I think, are:

  • Generate unit test stubs from examples (this is important to allow me to exercise the API)
  • Build a set of operations for fhir compliance
  • Work out what to do about extensions

I also think the basic datatype builders probably need work (and definitely testing).

@josephjclark
Copy link
Collaborator Author

I am not sure how to handle examples :(

In fhir 4, there is an 18mb zip file of examples. There are like 22 different patient examples. They do not have metadata attached.

Also, they're a different bundle, so it feels like a different command to generate these examples. And it's a lot harder to write into an existing test file.

All I really want is one or two test examples to include as stubs, and invite users to jump in and fill them out to exercise the API.

But looking at it like this, there's no way. It'll just have to be a manual process. Shame.

A good way to sketch out the value of the builder APIs. Not sure we're coming out ahead yet
@josephjclark
Copy link
Collaborator Author

Spend some time manually building out a Patient example today: see

const resource = b.patient({

The data structure is large, admittedly. But the builder so far only provides small gains. We can simplify things like codeable concepts. And we can take that further if we can cleverly integrate value sets (so we can map a code like M to a particular valueset and "expand it" into a full concept).

These small gains are lightly supported by docs, which roughly describe the properties of a patient (they're a bit coarse and they contain errors), and some code-complete, although this is disappointing (until/unless we swap out to typescript).

So at the moment, it feels a bit like the builders are generating thousands and thousands of lines of extra code for very minor benefits in user experience.

I am starting to doubt the benefit of his work. The builders are supposed to provide excellent, clean docs; great code-assist help to make creating data structures a breeze; and significant simpification of mappings (ie, remove the need to create verbose structures and manually import long system codes). We're not really there yet.

@josephjclark
Copy link
Collaborator Author

How much better would I feel if the generated adaptor was just SMALLER?

  • We generate builders for lots of things that I don't think would ever be built. Metadata stuff like ValueSets. Removing superflous builders would remove a lot of type defs, js code, tests, and docs content.
  • The actual builder code is pretty verbose. Maybe as much as 70% of code is simple property assignment which could be condensed right down
  • There's a lot of duplication between the d.ts files and the typedoc. The .d.ts file is nearly 24k lines long! I guess this is true of every adaptor

A smart prune would make me feel a bit better. The problem of "generating a lot of data for not much reward" goes away if we don't generate much data.

@josephjclark
Copy link
Collaborator Author

Ok, so I've removed INACTIVE profiles an FOUNDATIONAL profiles. That brings me from 329 files down to 268. That's about 20% less stuff. Not bad.

@josephjclark
Copy link
Collaborator Author

TODO: when using options like --simple-builders, save these to package.json. So that when we rebuild, those options are automatically re-applied.

Also, build:src is broken inside the actual adaptor since adding extra dependencies like stream-json

@josephjclark
Copy link
Collaborator Author

Spend a few hours today experimenting with generating TypeScript 🙀

Basically the problem is: when I'm generating js code, I have to bend over backwards to support code assist. And even then, it's not working very well and I can't take advantage of existing fhir typings.

So I'm trying to figure out of a minimal typescript implementation will produce a more robust type system. It should.

I'm about half-way through regenerating in typescript and it's looking promising. Should be able to report back tomorrow.

A lot of time is going into this. But it's worth remembering the goal here is first-class documentation and code assist while authoring fhir types. If we can't deliver that, then the utility of these auto-generated fhir adaptors is significantly decreased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: New Issues
Development

Successfully merging this pull request may close these issues.

1 participant