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.
✅ Deploy Preview for volview-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This was
linked to
issues
Sep 15, 2026
`npm ci` refused to install on this branch because the lockfile was missing the chokidar 3 subtree that vite-plugin-static-copy requires. Regenerated with `npm install`; `npm ci --dry-run` now passes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
The branch's own quality gate was failing. The duplication ratchet found six clones the branch added: four inside the DICOM chunk image spec, one shared by the two DICOM store specs, and one shared by the two new DICOM end-to-end specs. The complexity ratchet found that onRegularChunkHasData had grown to a complexity of 18 and that the new fillHoles algorithm was over both the complexity and the nesting limits. The chunk image spec now builds its held-open decodes with one deferredDecoder helper, shares the re-sorted chunk setup the three staleness tests all need, and asserts slice contents through expectSliceValues. The two DICOM store specs build their instance tags from a new shared fixture. The two end-to-end specs make their scratch directory with a makeTempDir helper in the existing test utils. No assertion changed. onRegularChunkHasData now delegates its three rejections to assertSingleFramePerFile, assertChunkFitsSlot, and assertSamplesRepresentable, which also removes the repeated "File X (chunk N)" prefix. fillHoles splits its per-slice flood fill into markOutside, fillEnclosed, drain, and fillBackground, so no block nests more than three deep. Both keep their behaviour and their error messages. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
The complexity, duplication, and convention ratchets only ran in the pre-commit hook, so a commit made with the hook bypassed reached main unchecked. The checks job now runs them after lint. They compare the checkout against the merge base with the target branch, which needs history the default shallow checkout does not have, so the checkout asks for the full history and the step fetches the base branch into its remote-tracking ref first. A pull request supplies its own base; the merge queue has none, so the step falls back to main. The scripts read the index and take HEAD as a commit, so the detached checkout a pull request build runs in needs nothing else. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
Restoring a state file mapped each saved annotation's imageID through the restore dataIDMap without checking that the image had actually loaded. When a dataset was unreachable its id is absent from the map, so its rulers, rectangles and polygons were seated with imageID undefined. No view can draw such a tool, and the next save wrote it back: the manifest's tools section then failed schema validation as a whole, so the annotations of the images that did load were dropped from the saved file too. Deserialization now skips a saved tool whose image did not resolve, so tools of resolved images survive a save unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
Converting an image to a labelmap resamples and decodes asynchronously, and the parent image can be deleted while that runs. Nothing rechecked the parent afterwards, so the split went ahead: it created a segmentation for the dead image, minted a registry segment and a mask record, and only then failed when the binding could not find the parent's voxels. The orphans stayed behind, naming a dataset that no longer existed, and every later session save reported missing parent content. The import now re-resolves the parent through the image cache after each await, before it decodes and before it splits, and the split itself refuses a parent that is not in the cache, so no identity is committed before the storage that would fail. Neither of the two live call sites awaits the conversion, so both now start it through startLabelmapConversion, which reports the failure through the message store the way other import failures are reported; the conversion progress flag was already cleared by the existing finally. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
Paint and polygon both resolved their edit target before asking whether the segment was locked. Resolving creates: it mints this image's mask record and, with it, the image's segmentation. A stroke or polygon refused for a locked segment therefore left an empty record behind, which is serialized into the state file and counts as a reference that blocks the segment's cleanup. The registry now answers where an edit would land without selecting or minting anything, and the segmentation store exposes that as editTargetLocked. Both edit paths ask it first, so resolveEditTarget only runs for an unlocked segment. The check stays beside resolveEditTarget rather than inside it, because that function's contract is to hand back a mask id and so it has to create one before it could report a refusal. The refusal messages are unchanged: paint stays silent and the polygon still reports "Cannot rasterize into a locked segment". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
The label path of ensureSameSpace only skipped vtkImageReslice when the product of the source worldToIndex and the target indexToWorld was a bit-exact identity matrix. Real geometry never satisfies that: with an origin and spacing such as [-120.1, -98.4, 33.7] and [0.7, 0.7, 3] the product carries float error of about 1e-5, so a labelmap that already sat on its parent's grid was copied through a full nearest-neighbour reslice on the main thread, once per imported or restored mask. The axis and eight-corner checks just above already prove the two grids coincide within the function's 1e-3 tolerance, so the identity test now only asks that the mapping neither permutes nor flips an axis, and the source image is returned as-is in that case. Permuted, flipped and genuinely different grids keep going through the reslice. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
Every segment actor is drawn at an opacity just under 1, so vtk.js puts it in the order-independent translucent pass with depth writes off. Overlapping segments therefore blend to the same colour whichever one comes first in the registry, and no amount of reordering makes one occlude the other. The README's "Ordering and overlap" section, the reorder handle's tooltip and the eyedropper comment all told the user that registry order decides what is drawn on top. They now describe what the order really controls: the sidebar, shortcuts, selection and picking, and flattened export precedence, and they say plainly that overlaps blend in the slice view. A new spec reads the README section and scans the segmentation and component sources so the claim cannot come back. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
Each segment actor asked for a coincident-topology polygon offset derived from its place in the registry, on the theory that distinct offsets keep coplanar masks from z-fighting and stack them for the viewer. That never worked. The offset only shifts gl_FragDepth, and a segment actor is translucent, so vtk.js draws it in the order-independent translucent pass, which writes no depth and weights fragments by the un-offset gl_FragCoord.z. The per-segment term changed nothing on screen. The shared part of the offset still matters, since it lifts the segments off the coplanar base image, so it stays as one constant. The stack index that fed the per-segment term is gone from the store, the slice viewer and the representation, and the specs that asserted it now assert the single offset and the registry order the move actually changes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
… count Splitting an imported labelmap was quadratic in its label count. Creating a mask walked the image's whole mask list to check the segment had no mask yet, picking a default mask-file name rebuilt a set over every bound mask in the scene, and minting a segment copied the entire segment record and the entire order array while searching for a default name it then discarded because the caller had stated one. Each segmentation now keeps a map from segment id to mask id, maintained by createMask and detachMask and read by maskFor. It is reactive, so a component that resolves its mask inside a computed still updates when one appears or goes. The names bound masks hold are kept in a set as bindings attach and rebuilt when a mask or a segmentation is removed. The registry mutates segmentById and segmentOrder in place, answers name lookups and both uniqueness scans from a name index, and searches for a default name only when the caller states none. Measured with a temporary spec splitting n labels onto a matching parent, before -> after: 250 labels 273 -> 95 ms, 500 828 -> 133 ms, 1000 2887 -> 235 ms, 2000 11073 -> 403 ms. Isolated at 2000 on a 4x4x4 parent: mintSegment 4514 -> 23 ms, createMask 1543 -> 42 ms, ensureLabelmapBinding 5794 -> 342 ms. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
Grouping masks into overlap-free layers asked every mask already in a layer whether it shared a voxel with the candidate, and that test sweeps the whole box the two masks share, stopping early only where it finds a shared voxel. Organ masks rarely touch but usually share a box, so the sweep ran in full for nearly every pair: the plan cost grew with mask pairs times shared-box voxels, on the main thread, and both the save dialog and the job input warnings re-run it from a computed. A layer now keeps an occupancy buffer, one bit per voxel of the box taking in every mask being grouped. Each accepted mask is written into it once, and a candidate is answered by a single sweep of its own extent. A layer holding a single mask is still asked that mask directly, so a scene where every mask overlaps every other allocates nothing. The layering itself is unchanged, and masksIntersect stays for its other callers. Measured with a temporary spec, since deleted: 100 masks holding disjoint voxels in overlapping 128x128x32 boxes on a 256^3 parent took 2781 ms before and 241 ms after. The same 100 masks all claiming one shared voxel, where no layer ever takes a second mask, took 292 ms before and 263 ms after. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
A segmentation record can exist with no masks at all: dragging a display slider mints one for an unpainted image, and deleting the last segment leaves the record behind. The labelmap binder only checked that such a record was attached to the active image, so the input reported itself bound and Run staged a full parent-shaped, all-background file. The staging context now passes the segmentation only when it holds at least one mask, which is the single point both the binding and the staging paths read, so an empty record falls to the existing no-segmentation branch and nothing is staged for the parameter. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
…olds The selected segment is shared across images, so it can name a segment that has no mask on the image being packed. Single-file packing passed it straight through as the preferred segment anyway, so the greedy fill fell back to registry order while the warning still told the user to select a segment to prioritize it, which they had already done. Planning now only honours the selection when this image holds a mask for that segment, and the warning offers the advice only when no segment on this image is selected, naming the image so the advice is actionable. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
…pported An annotations result declares each label's colour as a CSS string, and the applier pushed that string through cssColorToRGBA, which falls back to opaque black for anything it cannot read. The parser accepts hex, the CSS colour keywords and `transparent` by design, so a task emitting rgb() or hsl() had its labels silently minted black with nothing said about it. Binding now parses with tryCssColorToRGBA and simply omits the colour when the parse fails, so the segment keeps the colour it already had or the automatic palette colour a new one is minted with. Every rejected label is collected across the three tool kinds and reported once, naming the label and the value it stated, in the same wording an imported config uses for the same mistake. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
The `multiple` flag's description claimed the client stages the whole segmentation as one flattened labelmap file whatever the flag says. It does not: a labelmap sourceRef stages overlap-free parts, one staged file each, and the flag decides how many of them are sent. The text now says that true sends every mask across as many files as overlap requires, that false sends only the non-conflicting subset that fits one file and omits the rest whole with a message to the user, and that no mask is clipped. The annotations label colour was an undocumented open string, so a producer had no way to know which syntax the client reads. It now states that a hex value or a CSS colour keyword is accepted and that anything else is ignored, leaving the label the colour the client already holds. The negative-fixture rule promised more than the published schemas deliver: wrong-length-color.json is rejected only by the strict known-intent union, since the published result-intent schema is deliberately open. The README now records that exception where the rule is stated. Generated artifacts regenerated with npm run contract:generate; only these three descriptions changed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
…selection The Jobs panel watched the task model and the selected segment together and refreshed the input warnings without a debounce, while the line below already refreshed them from the debounced overlap signal, which itself carries the selected segment. Every selection change therefore ran the whole mask overlap sweep twice. The undebounced watcher is now narrowed to the task model, so a task change still refreshes immediately while selection changes are covered once, by the debounced watcher. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
Auto-loading skipped any job result whose declared intent the applier could not route, with a bare continue. Nothing reached the user: the completion toast had already announced that the results were ready, and the Jobs panel still offered the file as a plain download, so a result produced by a backend that names an intent this client no longer knows, such as the pre-rename add-segment-group, simply never arrived in the scene. The skip now surfaces a warning naming the result and the intent it declared, and pointing at the download that is still available. Results that declare no intent at all stay quiet, since there is nothing to route for them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
…belmap Staging labelmap inputs asked the edits coordinator for a committed read before the loop over the task's labelmap bindings, and that read cancels an unconfirmed segmentation preview. The loop can iterate zero times, so submitting any task at all, including one that takes no segmentation, discarded whatever preview the user was still deciding on. The read is now requested only once there is a binding to stage, so a task with no labelmap binding leaves the preview standing while one with a binding resolves it as before. The shared staging scene the specs seat moves into a helper so both cases can reuse it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLyi9JtZ3Bxq1vy96ZJCob
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
2 participants
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.