Split AREG into three tools, fix what the splits left stranded, and repair CI - #21
Merged
Conversation
…free common package
…y and orphaned semaphore
…uite to the split API
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.
What this does
Completes the AREG split (AREG_CBCT / AREG_IOS / AREG_IOSCBCT over a shared
common), and fixes what both this split and the earlier ALI one left behind.The defects the splits left, and why nothing reported them
Splitting a tool leaves its old
tests/test_run.pyimporting the pre-splitpackage. pytest reports that as one collection error, so the whole file stops
running: 84 tests for AREG, 16 for ALI. One error reads as one problem.
Behind it:
AREG_IOSasked the supervisor forALI, renamedALI_IOSby the split.The mucogingival mode could never have run. The server's startup check missed
it because
describe.pyscannedsup.run("X")and nottools.require(sup, "X", ...)— the hole that function's own docstring warnsagainst. Both are fixed, with tests.
ALI_CBCT's error message namedcbct_regions, an argument the split hadrenamed to
regions, so it told callers to fill a field the schema does nothave.
GET /tools/AREG/data, a 404 since thesplit.
ASOreachedsplit_scan_extensionthrough the CBCT pipeline rather thanits own
scansmodule.CI
find -mindepth 2 -maxdepth 2, which stoppedmatching when ALI and AREG became grouping folders. CI has been testing six
tools and silently skipping the five the splits produced — which is where the
stale tests were found, months later, by hand. It now walks depth 2 and 3 and
keys on
[tool.sadt], the same rule the registry and the deploymentDockerfile use. That also drops
_template, which CI tested and no serverserves.
actions/checkoutandastral-sh/setup-uvmoved to the lowest majors runningon Node 24, clearing the deprecation warning.
Verification
Every suite, run in its own interpreter:
All ten tools were also run end to end over HTTP against the deployment image,
through the Slicer client, on real data — including the three-level supervised
chain (AREG_IOSCBCT to ASO to ALI_CBCT).
AREG_IOSCBCTkeeps only one test of its own; the rest of the merged suitebelonged to the other two. Its own argument rules deserve a suite and do not
have one yet.