Skip to content

Conversation

@darenkeck-dev
Copy link
Collaborator

@darenkeck-dev darenkeck-dev commented Jul 29, 2025

Description

Server updates to support nested SystemTypes.

  • Adds the parent attribute to SystemType
  • Updates SystemType extraction process to populate the parent field
  • Adds tests for SystemType extraction
  • Update tsconfig target and bump typescript version

Client

  • Add business logic to generate nested structure of templates
  • Update tests for schema updates
  • Update tsconfig target and bump typescript version

NOTE: no visible updates have been made in the client UI. This PR merges work to prepare for displaying nested templates and system type groups.

Related Issue(s)

#455

Testing

Tests have been updated on the server

Copy link
Collaborator

@AntoineGautier AntoineGautier left a comment

Choose a reason for hiding this comment

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

@darenkeck-dev See my comments:

  • The ES version should be updated if we want to use the at()method.
  • The other one is more a question related to the requirement to organize templates by library.

@darenkeck-dev
Copy link
Collaborator Author

@AntoineGautier changes have been made hiding any visible update in the client UI. This is ready for review again.

@AntoineGautier AntoineGautier force-pushed the dk/455-nested-systemtypes branch from 07e0b44 to 2872fce Compare January 5, 2026 16:13
@AntoineGautier
Copy link
Collaborator

AntoineGautier commented Jan 5, 2026

@darenkeck-dev Tests failed with 2872fce because of the following break in the JSON file path discovery:
https://github.com/lbl-srg/ctrl-flow-dev/blob/main/server/src/parser/loader.ts#L180-L186
Do you remember the rationale to have getPathFromClassName return null in case typeStore.has(modelicaPath)?
This seems like breaking the proper separation of concerns between:

  • the function getPathFromClassName that should always try to return the path to the JSON file if it exists,
  • any caller of getPathFromClassName that should check that the file has already been loaded before loading it again.

I would be willing to remove this check from getPathFromClassName (for now, I've just moved it after the package path discovery). What do you think?

Copy link
Collaborator

@AntoineGautier AntoineGautier left a comment

Choose a reason for hiding this comment

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

(I pushed a fix for stopping the discovery process after processing the root package.)

Regarding the changes from dk/455-nested-systemtypes, I tested loading two test packages with server/scripts/parse-test-package.ts and validated that configuration parameters specified in the UI are properly included in the payload with getSequenceData().
I also tested that dk/455-nested-systemtypes produces the same sequence document as the main branch for the same set of parameters.

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.

3 participants