Conversation
Baseline on branch segment-model, cut from main at f482c6d. Unit suite: 97 passed, 1 skipped file (955 tests: 946 passed, 9 skipped); the skipped file is the pre-existing src/core/thumbnailers/__tests__/vtk-image.spec.ts. Lint and typecheck: pass. Inventory of useSegmentGroupStore|activeSegmentGroupID|activeSegment over src (excluding __tests__): 27 files, 111 sites. Heaviest files: 32 src/store/tools/paint.ts 14 src/store/tools/paintProcess.ts 5 src/store/tools/fillHoles.ts 4 src/processing/applyResults.ts 4 src/io/state-file/serialize.ts 3 src/utils/bugReport.ts 3 src/store/segmentGroups.ts 3 src/processing/engine/mintLabelmap.ts 3 src/processing/composables/useInputStaging.ts 3 src/processing/components/JobsModule.vue 3 src/components/tools/polygon/PolygonTool.vue 3 src/components/tools/paint/PaintWidget2D.vue 3 src/components/SegmentList.vue 3 src/components/SegmentGroupControls.vue
Exports a segment group before and after a rename and recolor to show the voxels and the label value stay put while the name and color follow, and drives the cross-image clone: viewing another image gets nothing, its first edit clones the active segment's name and color into a segment of its own, and later edits land on the segment each image already has.
… none A polygon drawn before any segment exists carried no label, and rasterizing threw. Nothing seeds a segment without config labels, so a stock build had no active segment on a freshly loaded image. Rasterizing is itself an edit, so it now resolves the edit target.
startProcess resolved the edit target before checking whether the process needs an active segment. resolveEditTarget is the one call that creates segments, so an all-segments process minted a default segment on a bare image, or cloned the active segment onto an image the user had only viewed.
…ckstop The store registers an onImageDeleted cascade with no matching declareManifestRefs, against the contract in core/manifestRefs.ts, so the dev-only orphan backstop covered no segmentation state despite the manifest carrying resolvable segmentation and segmentationArtifact kinds.
The Rasterize menu item was disabled whenever the polygon had no label, so the default-segment path was unreachable from the UI. The polygon now records the segment it rasterized into.
serializeIdentity filtered props by the segments finished tools referenced, so a label customized before any annotation was placed lost its tool properties on save. It now filters by whether the segment is still live.
activeSegmentId returned the active target's segment whatever image it belonged to, so an annotation placed after switching images carried a segment its own image's segmentation does not hold. It was then absent from the segment list and rasterizing it threw.
serializeIdentity and adoptIdentity advertised parameters neither implementation read. listSegments and NO_NAME each had two definitions, and bindingsForArtifact scanned every loaded image to answer a question about one artifact's own segmentation.
shouldPaint runs per candidate voxel, roughly 81 per brush point at size 4, and re-read the labelmap scalar buffer on every call. Both buffers are fixed for the stroke.
activateTool called resolveEditTarget and ensureLabelmapBinding, so selecting the brush created a segment and allocated a labelmap before any stroke, against the invariant that only an edit creates. doPaintStroke already resolves at stroke time. The e2e encoded the old behaviour by activating paint without painting; it now paints a stroke.
ProcessAlgorithm took (segImage, labelValue), so fillHoles scanned artifactIndex by object identity to recover an artifact id startProcess already held. It now receives the resolved target, and findArtifactIdForLabelmap is gone. startProcess also resolved a segment for all-segments processes, which need an artifact and ignore labelValue. Segment-scoped and artifact-scoped resolution are now separate, and only the segment-scoped path creates.
resolveEditTarget now takes an optional preferred segment, so rasterizing a labeled polygon no longer builds its own target. A stale id left on a tool whose segment was deleted falls through to the default segment instead of throwing; a live segment owned by another image is still an error. clearDefaultLabels was a no-op while configJson still called it, so loading a second config left the first config's seeded labels behind.
The brush transforms came from the globally active artifact, so after switching images the widget installed the previous image's index and world transforms and the first stroke landed displaced.
Artifact pruning dropped artifacts with missing bytes, but segment validation only checked schema shape and parent dataset, so the normalized manifest kept bindings to artifacts it had just removed and restore silently recreated those segments with no storage.
Two reviewers proposed merging this with the segmentation store's cursor. Both are separate on purpose, and each resets with the pinia instance, which is what keeps a descriptor-less labelmap decoding to the same catalog from a cold restore and a live conversion.
The label was written back only when the polygon carried none, so a polygon whose segment had been deleted kept the dead id while its voxels went to a replacement segment, and restored unlabeled.
…them Config labels accept any CSS color string, but only hex and the sixteen basic keywords were recognised, so rgb(), hsl() and transparent silently became opaque black. tryCssColorToRGBA now reports unparseable input to callers.
…ve one Migration built segment ids by joining legacy identifiers with '-', which those identifiers may contain, so distinct sources could collide and restore, keyed on that id, silently misrouted one onto the other. A descriptorless legacy group emits no segments at migration time, so its active paint value had nothing to match and was dropped. The value now travels on the artifact and restore reactivates it once the decode has created the segment.
PaulHax
force-pushed
the
segment-model-quality-check
branch
from
September 20, 2026 13:08
dcc9616 to
e141dc8
Compare
The two scripts landed before lint-staged covered .mjs files, so the hook never formatted them. Formatting them now keeps the next touch from mixing a reformat into a real change.
The in-flight conversion was recorded under the child image alone, so a second call naming the same child but a different parent joined the first conversion and was resolved with masks created on the other parent, while its own parent got no segmentation and nothing reported a problem. The map is now keyed on the child and parent together, so only a caller asking for the same pair joins; a conversion onto another parent runs as its own work. The pending flag the browser reads stays keyed on the child image.
…e key The duplicate-id guard in adopt tested `id in idMap` against a plain object literal, so a wire id such as 'constructor' or 'toString' read as already seen: no segment was minted for it, and because the returned map held no own property for that key either, a consumer looking it up got an inherited function instead of a miss. The map is created without a prototype and the guard asks for an own property, so those ids are ordinary ids again on both sides.
The binding pass returned early, and silently, for a saved segmentation whose parent image has no entry in the restore's id map, so every mask of an image that failed to import disappeared without a line in the report: losing masks read exactly like having none. An unmapped parent is now treated as an unavailable one, so each of its masks reaches the report with the reason the other unavailable-parent drops already use.
The seg.nrrd header path appended the values it declared but this component's voxels missed on every component, so a two-component file whose value 1 sits in component 0 alone came back as Liver, Spleen, Liver (2) and Spleen (2): an empty twin of a segment that has voxels, which the result path already forbids. The decode is told the values the file's components carry and which component is the last, so a declared value another component carried stays that component's segment and one no component carried becomes a single empty row. The comment and the README say the two paths agree again.
Every labelmap a restore reads looked its archive member up by scanning the whole file list, normalizing the path of each entry it passed, so a session with many masks walked the archive once per mask. The members are keyed by their normalized path once, before anything is read, and both lookups ask that index. A key two members share still resolves to the first of them, as the scan did.
…review Saving a session cancels a preview holding live mask storage so the file gets committed content, but that call was the one caller of the contract with no test behind it: removing it left every suite green, and no end-to-end spec combines a preview with a session save. The new spec holds a cancel the way the staging spec does, saves a scene with one bound mask, and asserts the preview was cancelled once and the mask still reached the manifest.
The export plan records overlap and the label-value limit separately so the save dialog can say which one split a segmentation into several files, but no spec read either flag: merging them into one boolean, or deriving the capacity flag from the part count, left every suite green. The grouping spec now compares the whole plan shape for an overlapping pair and a disjoint pair. Reaching the capacity limit needs 65535 masks, so only its answer of no is pinned, and no production seam was added for it.
The concurrency spec restated the cap as a literal 4 of its own, so changing the production limit to any other number would leave the spec asserting an old value and failing for the wrong reason, or passing while nothing was bounded. The cap is exported from the module that applies it and the spec asserts against that constant.
…asks Every import fixture carried 8-bit scalars, so nothing tested the rule the README states: a 16-bit labelmap keeps its labels until it is split, and the masks it becomes are binary. Truncating a label into a byte on the way in went unnoticed. The split spec now imports a labelmap whose scalars are 16-bit and whose label values pass the byte limit, and asserts the value reaches the decoded name and the masks it splits into hold byte-sized voxels.
…file A saved labelmap read straight from the archive was decoded with no name to fall back on, so its segments came back as 'Segment 1', 'Segment 2' while the same labelmap converted live, or restored through a loaded dataset, was named after the file it arrived in. The decode now takes a base name from the caller when no loaded image can give one, and the restore passes the labelmap's own name. The parity spec compares the archive-backed catalog with the live one whole, rather than pinning the name that said nothing.
The new prototype-key case reached the minted id through a property whose apparent type is Function, so the cast to string it needed failed the type check. It reads the one value the map holds instead, asserting the same thing with no cast at all.
masksHolding and masksClearing.claim swept the reaching masks with some/forEach, so every voxel a brush or polygon fill walked allocated a fresh callback capturing i, j and k. Both now sweep the reaching masks with a plain indexed loop, so the per-voxel path allocates nothing. The answers are unchanged.
A stroke turned every point the brush handed back into the parent frame with toParent, which built a three-element array; shouldPaint and onPainted each called it, so a mouse-move sample allocated two arrays per stencil voxel. The mask origin is now read into three constants and applied a component at a time, so the underlying-pixel lookup and the neighbour claim take the same parent indices without allocating. Painted voxels are unchanged.
When an algorithm rejected, the catch block wrote a snapshot back into every target and called modified() on it, including masks the run never built a run for and never touched. An all-segments process that fails outright therefore invalidated every mask's slice representation for a no-op. Rollback now skips a target with no run: its storage still holds what the user left there. A spec covers a failing algorithm on two targets and pins the untouched mask's buffer identity and modified time.
…ning it Fill Holes and Gaussian Smooth posted the mask buffer to their worker as an ordinary argument, so the structured clone at the boundary allocated a second copy of every target mask on top of the snapshot the manager keeps and the detached copy it hands the algorithm. Both calls now wrap the payload in Comlink.transfer and list the buffer, which moves it to the worker. The buffer is the manager's own detached copy, built for this call and read by nothing afterwards, so nothing on this side is left holding it. The Fill Holes spec pins the move, and its comlink stand-in gained the transfer the real module provides.
The stroke was converted into the mask's own index space before the brush walked it. That walk accumulates fractional steps and rounds, so where it starts decides which side of a half a step lands on: the same world stroke painted a different edge voxel depending on where the mask's storage happened to begin. A mask covering the whole parent and a freshly grown one differed by a voxel on a plain six-step diagonal. The brush now takes the points on the parent grid and an origin saying where the labelmap's first voxel sits in it, offsetting only when it indexes the buffer and tests bounds. A mask allocated at offset zero paints exactly what it painted before, and the callbacks now read parent indices directly. A spec paints one stroke into a whole-parent mask and a bounded one and compares the voxels.
…to do A mask whose allocation holds none of the label came back from the filter as a full copy of itself, so the process manager built a run, rewrote identical bytes, invalidated the renderer and dropped the user into a preview whose Original and Processed are the same picture. The filter now returns nothing for that mask, which is the manager's existing "nothing to do": the run is dropped and an all-empty pass warns instead. A mask that holds the label is unaffected, and the golden output is unchanged. The specs that read the result directly say so, and two tests cover the empty case: the filter's answer and the process ending at its start step with the warning.
A mask record is minted the moment a segment is resolved as an edit target on an image, and allocating its storage does not put a voxel in it. Two readers took the record for content. The delete message counted every record naming the segment, so deleting one reported a mask deleted on an image where nothing had ever been painted, while the same list showed that row's Reveal disabled with "This segment has nothing on this image". The Save gate asked only whether the viewed image's segmentation had any mask in its order, so a record with no voxels offered a save that would write nothing. Both now ask one predicate, maskHasContent on the model: storage bound over a non-empty extent. The segment list's own bound-mask lookup, which already spelled that rule out, uses it too. The spec that pinned the old count built its painted segment from a bare record, so it now seeds a voxel to be what its name says, and a new case covers a segment that was only resolved on an image. The save specs that needed something to save seed a voxel for the same reason, and a new case holds the gate closed for a record and for empty storage.
The save dialog listened for Enter on the window, so any keystroke reached it: opening the format menu with the keyboard and pressing Enter to choose an option also started the export, with the format the select had not applied yet. Nothing checked whether a save was already running either, and one keystroke arrives twice when the form submits as well, so a second run composed the same masks again and downloaded the archive a second time. The listener now sits on the dialog's card, so it hears the keystrokes that belong to this dialog and not the ones an overlay above it owns, and the save returns early while one is in flight. A new spec mounts the dialog attached to the document and stops the save where it hands its parts over, so it can count the runs one keystroke starts.
The case for the coincident-topology offset asserted only that the pair has two entries and that both are finite, which holds for any pair of numbers: replacing the constant with an asymmetric [-1, -999] left the whole file green. It now pins the value the lift off the base image was measured at and that both entries are the same number, which is what "one offset, no per-segment term" means. The mutated constant fails it.
resultToIntent returns undefined for two different reasons: an intent name this client does not know, and a known name whose payload fails the strict shape union. The skip notice described only the first, so a result naming a fully supported intent with a malformed payload told the user their VolView was too old to apply it. The notice now checks the name against RESULT_INTENTS: an unknown name keeps the version wording, a known one says the result does not carry the payload that intent requires. A case beside the unknown-intent one drives an import-segmentation result with a three-component color through the loader.
The editing and persistence section still said job input staging cancels an unconfirmed preview, which stopped being true once the committed read moved behind the check for a labelmap binding: a task that stages no labelmap now leaves the preview standing. The sentence now names the condition and says what happens otherwise. Save and export are unchanged.
…ring The segmentation staging cases spy on planLabelmapExport without restoring it, so the spy and its tally survive to the end of the file. One case cleared the tally by hand, the other did not, and 'does not rescan the masks when a stroke grows one' passed only because it sits first; swapping the two cases made it fail with two recorded calls. The describe now restores mocks after every case and the ad-hoc clear is gone. Its debounced waits move to fake timers, so the cases step past the 150 ms overlap debounce instead of sleeping 300 ms; with the debounce widened to 500 ms they still fail, so they keep pinning it.
The pure-layer zone derived './index', '../index' and '@/src/<dir>/index' for the index upper module, but not '.', '..' and '@/src/<dir>', which resolve to the same file. A pure file could import its own feature entry point, and with it the store, vue and the boot-time registration that entry point runs. Those three spellings now join the rule as exact paths whenever the feature lists index among its upper modules. They are exact paths rather than pattern entries because the pattern '..' matches every '../sibling' below it too, which would deny the pure-to-pure imports the layer is built on. The shared message moves to a helper so both halves of the rule read the same.
Polling, cancel and result loading all classify their errors and hand a 401 to markSessionExpired. The job-history load did not: it logged and recorded a per-provider string, so the session stayed live in the store, the persistent reload notice never appeared, and the panel offered a Retry that could only 401 again. Its catch now goes through expireSessionIf first, and both the history request and the page loop stop once the session is expired, so nothing re-issues a request that cannot succeed. The two catches build their detail string in a local, which keeps the file inside the line ratchet. A case in the store spec drives a 401 listing through adoption and asserts the notice, the empty history error, and that a retry sends nothing.
The page's only worked example was the 6.2.0 manifest, relabelled as legacy but never joined by the schema a current session writes, so an integrator emitting *.volview.json had nothing to copy but a form with segmentGroups and per-tool labels that no build produces. A 7.0.0 example now leads the section with segmentations, segments and masks as src/io/state-file/schema.ts defines them, including the artifact a sparse manifest uses to point at a remote label volume. The legacy example keeps its place under a heading that says it is the pre-7.0.0 form.
…t lacks A segmentation result is recognized as already applied through the `source` the producer stamps on it, and that field is optional on the wire. A backend that omits it left the masks with no receipt, so after a reload the re-adopted job's Load button imported every mask again, arriving as Tumor (2), Tumor (3) and so on. The client now mints the key when the result carries none, from the provider and job it submitted plus the result row's id. Nothing new goes on the wire: the minted key is scene provenance, stored on the mask binding and restored like a producer's own, and a producer-sent source still wins. A case applies a source-less result, hands its receipt back as a restore would, and shows the second apply converts nothing.
This branch has not been deployed
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
1 participant
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.
Introduces a segment model with updated editing, rendering, processing, and state-file workflows. Adds segmentation controls and coverage for import, persistence, and user interactions.