Skip to content

fix: the pre-tag pass over what 1.8.0 ships - #90

Merged
vboussot merged 4 commits into
mainfrom
fix/pre-1.8.0-review
Aug 4, 2026
Merged

fix: the pre-tag pass over what 1.8.0 ships#90
vboussot merged 4 commits into
mainfrom
fix/pre-1.8.0-review

Conversation

@vboussot

@vboussot vboussot commented Aug 4, 2026

Copy link
Copy Markdown
Member

A last read of everything between v1.7.0 and the tag. Twenty commits: what would have shipped
wrong, one operator that was missing, the flagship feature's first example, and the release
machinery that could not do what three places said it did.

What would have gone out wrong

The release notes. The v1.8.0 section was written three commits before the tag and never
caught up: it advertised ShapeUpdate, which the compose pass removed two commits later, and
omitted ResampleToReference and the composed pass — the two largest features in the release.

konfai-mcp named konfai with no bound while runner.py imports konfai.transformer at
module scope. Installed beside a 1.7.0 konfai, that kills the whole server on import, not just the
transform tools. Spelled >1.7.0, not >=1.8.0: setuptools_scm builds the unreleased tree as
1.7.1.devN, which PEP 440 sorts below 1.7.1, so a floor at the coming release is
unsatisfiable from a source checkout — CI caught that on the first push.

A uint8 label map resampled through a field was interpolated and truncated. _stream_mode
already answered nearest for uint8, but only the two paths without a field ever consulted it.
Over a source holding {0, 100} the warped path returned 29, 79 and 99 — labels nobody wrote, in a
volume that is still a valid label map. ResampleToReference takes an interpolation now, as
Warp already did.

A statistic after a Reduce was seeded from the fold, so [Reduce, Clip, Normalize] normalised
by the unclipped Max and wrote a volume peaking at 0.48 whose own header described something else.
The per-case planner already refused this; the reduction does too now.

An OME-Zarr chunk was a gigabyte at 2048² and past zarr's single-buffer ceiling at 4096²,
because the writer's declared region was taken verbatim. An axis the region covers end to end can be
tiled without ever splitting a region write; the axis the writer advances along keeps its height.
Appending pyramid levels also rewrote level 0 on ngff-zarr's default chunking, so a streamed write
and a declared pyramid cancelled each other out.

Warp raised on a GPU-resident volume (grid is on cpu, different from other tensors on cuda:0), verified against the pre-fix revision on a real device.

from konfai.data import Clip — the first line of the public API example — raised ImportError.

Behaviour changes worth reading

Median moved from torch.median to torch.quantile in a refactor: commit, which changes what
an unchanged PREDICTION config writes: an even count now averages the middle pair ([1.0, 3.0] gave
1.0, gives 2.0), and an integer input widens to float32. That is correct for intensities and
wrong for labels — the median of 1 and 5 is 3, and over exactly two cases Median is Mean. So
Vote is added: it takes the label the most cases agree on, keeps the dtype, and breaks a tie
toward the smallest label. Median's docstring and the guide say what it is not for.

The CHANGELOG carries a ⚠️ Behaviour changes section for these and for the four new refusals.

The example

examples/Transform/ — the release's headline feature shipped without one, and the konfai-cli
skill still described three workflows. Both cardinalities: a median template folded from a cohort
that shares no grid (N→1, which is what ResampleToReference is in the chain for) and four drawn
copies per case (1→N). Six volumes generated locally, 3.5 MB, nothing downloaded, a minute on CPU,
written uncompressed because a compressed .mha cannot serve a disk region. Both configs are run
as shipped by an integration test.

Release machinery

The workflow re-rendered the release notes from the commits while the committed section is a draft
that was then edited, so the file and the release page were guaranteed to differ — the opposite of
what publish.yml, development.md and CHANGELOG.md all claimed. It takes the committed section
verbatim now, and a tag with no section fails the job.

Every action is pinned to a commit SHA (40 references, 7 workflows), there is a contents: read
floor, the jobs that run third-party code no longer persist the token, and pre-release detection
matches the version instead of searching the whole tag for a letter — latest on Docker Hub moved
for any tag at all.

Refactor

transformer.py reached into three private names of patching; they are that module's contract
with the workflow and are spelled as one. _plan_stream_region returned the same five-tuple six
times, so each branch is now the sentence it refuses with.

The three samplers shared one arithmetic written out at each site, and it had already drifted
the CPU-half guard existed in three of them and not the fourth. sampling_dtype, nearest_index
and window_index state each rule once. The two gather strategies stay: per-axis maps where the
coordinate is separable, eight flat corners where a displacement makes it not.

Not done, and measured rather than assumed: extracting the 1→N expansion engine out of patching.py
would need ~20 of DatasetManager's private members made public — its nine methods touch 35, where
CaseReduction reaches a manager through 7, all public. The asymmetry is real, not misplacement.

Verification

  • pytest tests/ studio/tests konfai-mcp/testsexit 0, 1681 tests, zero FAILED/ERROR
  • sphinx-build -Wexit 0, zero warnings (two existed before this branch)
  • ruff check clean; mypy and bandit green on every commit through pre-commit
  • The sampler merge is bit-identical to the revision before it: 36 arrays over 2 value ranges ×
    6 dtypes × 3 sampler paths
  • Warp on CUDA verified on a real device, and confirmed raising on the pre-fix revision
  • New tests: the two gathers agree bit for bit through an identity field; the sampler rules against
    SimpleITK's own interval; label-map interpolation against sitk.sitkNearestNeighbor; Vote;
    the chunk size a declared region turns into; the shipped example, run as shipped

Note on scope

This is not one logical change, which AGENTS.md asks of a PR — it is one pre-tag pass, grouped
into four commits by area: the core package, the example, the Studio/MCP surface, and the release
machinery. They are checkpoints of a single pass rather than four independently releasable steps,
so review the diff rather than bisecting it.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added the CPU-only TRANSFORM workflow for dataset processing without model training.
    • Added Vote reduction for label maps, reference-grid resampling, and configurable interpolation.
  • Improvements

    • Enhanced planning, validation, memory handling, dtype support, geometry checks, and OME-Zarr output.
    • Improved Studio protection for shutdown requests forwarded through untrusted proxies.
    • Strengthened release automation and Docker image builds.
  • Documentation

    • Added Transform guides, examples, and demonstrations.
    • Expanded guidance for reductions, interpolation, planning, and label-map processing.

Corrected while this was open

Two claims did not survive being measured, and the code went with them:

  • A sidecar written at finalize was said to need re-consolidating for max_displacement: auto to
    read it back. It does not: a bound written through an r+ group is read back by konfai's own
    reader and by a foreign consolidated zarr.open_group, on both sides of the change. The extra
    consolidate_metadata was a metadata rewrite per field finalize that bought nothing, and is gone.
  • The konfai bound was >=1.8.0, which no source checkout can satisfy. See above.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • ✅ Review completed - (🔄 Check again to review again)
📝 Walkthrough

Walkthrough

This PR adds the TRANSFORM workflow and examples, updates reduction and resampling behavior, improves OME-Zarr chunking, hardens Studio shutdown handling, pins CI actions, and changes release notes to use validated changelog sections.

Changes

KonfAI workflow and runtime updates

Layer / File(s) Summary
Transform workflow integration and examples
.claude/skills/konfai-cli/SKILL.md, konfai/main.py, konfai/transformer.py, examples/Transform/*, tests/integration/*
The CLI registers TRANSFORM. Examples cover cohort reduction and four-copy expansion. Integration tests execute both workflows and verify their outputs.
Reduction operators and planning
konfai/data/reduction.py, konfai/data/case_reduction.py, konfai/data/data_manager.py, tests/unit/test_case_reduction.py
Vote performs per-voxel majority selection with smallest-label tie-breaking. Reduction planning accounts for operator workspace, source channels, locality, and missing geometry.
Patch-stream state and destination resolution
konfai/data/patching.py
Fallback constants and destination resolution are public. RNG snapshots include CUDA states. Failed stream plans and Save sweeps preserve their original failure state.
Centralized resampling utilities and contracts
konfai/data/transform.py, tests/unit/test_resample_sampler_rules.py, tests/unit/test_resample_to_reference.py
Sampling uses shared dtype, rounding, indexing, and interpolation rules. ResampleToReference accepts explicit interpolation. Warp diagnostics and device handling are updated.
OME-Zarr chunking and metadata
konfai/utils/dataset.py, konfai/utils/ome_zarr.py, tests/unit/test_ome_zarr_data_surface.py
Chunk sizing uses dtype-aware byte limits and preserves declared partial regions and level-0 pyramid chunking.
Studio proxy-aware shutdown
studio/konfai_studio/cli.py, studio/konfai_studio/server.py, studio/tests/test_quit.py
Forwarded shutdown requests require trusted proxy headers. Delayed shutdown tasks remain tracked until completion.
CI workflow pinning and trigger updates
.github/workflows/*
GitHub Actions use immutable commit SHAs. Checkout credentials are not persisted. CI triggers include Transform and configuration-guide fixtures.
Release automation and changelog validation
.github/workflows/publish.yml, CHANGELOG.md, docs/source/development.md
Release notes come from a validated changelog section. Stable versions use numeric matching. Docker tags use PEP 440 patterns and conditionally add latest.
Documentation and API reference
docs/source/config_guide/transform.md, docs/source/reference/*, examples/*, konfai-mcp/*, konfai/data/__init__.py, konfai/predictor.py
Documentation covers Transform planning, reductions, interpolation, patch locality, examples, MCP outputs, and public reduction exports.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant KonfAI_CLI
  participant Transformer
  participant DataPipeline
  participant OutputStore
  User->>KonfAI_CLI: Run TRANSFORM or --plan
  KonfAI_CLI->>Transformer: Load Transformer configuration
  Transformer->>DataPipeline: Apply resampling and reduction stages
  DataPipeline->>OutputStore: Probe or write transformed data
  OutputStore-->>Transformer: Return plan or output status
  Transformer-->>User: Report plan or completed transform
Loading

Poem

A rabbit hops through grids so wide,
With Vote to choose and Plan to guide.
Pinned actions guard each nightly build,
Chunks stay bounded, buffers skilled.
Studio's quit checks proxy hands—
Clean transforms bloom across the lands.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 61.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the PR as a pre-tag review of what will ship in v1.8.0.
Description check ✅ Passed The description thoroughly explains the changes, rationale, scope, and verification, although it does not use all template sections.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pre-1.8.0-review

Comment @coderabbitai help to get the list of available commands.

@vboussot
vboussot force-pushed the fix/pre-1.8.0-review branch 2 times, most recently from 555d210 to 2b715b0 Compare August 4, 2026 13:38

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 11

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
konfai/data/data_manager.py (1)

1979-1990: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not omit terminal Write destinations from validation.

_output_destinations() records a Save only when transform.dataset is truthy. This excludes Write: {}. However, save_destination() in konfai/data/patching.py resolves a missing dataset to the manager's dataset. Because _validate_write_chains() accepts a terminal Write without a dataset, the source collision check and duplicate-target check do not see this destination.

A Write: {} configuration can therefore skip existing source entries or overwrite the source when --overwrite is enabled. Resolve every prepared destination through save_destination(), or reject a terminal Write without an explicit dataset.

One safe validation fix
             if not transforms or not isinstance(transforms[-1], Write):
                 ...
+            if not transforms[-1].dataset:
+                raise TransformerError(
+                    f"'{chain}' ends with a 'Write' without a destination.",
+                    "Give the terminal Write its own dataset; a transform must not write into its source.",
+                )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@konfai/data/data_manager.py` around lines 1979 - 1990, Update
_output_destinations() so terminal Write/Save transforms without an explicit
dataset are included in validation by resolving their destination through
save_destination(), using the manager’s dataset fallback consistently. Ensure
the resulting resolved path and group are added to destinations so
_validate_write_chains() performs source-collision and duplicate-target checks
for Write: {} configurations.
konfai/data/case_reduction.py (1)

88-108: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Account for input and output region sizes separately.

For Concat, region_bytes uses N × C output channels. resident_regions then charges each of the N source buffers at that output size. The new working_multiple = 1.0 adds more output-sized buffers. This can greatly overestimate memory, reduce slab size, or refuse a reduction that fits the configured budget.

  • konfai/data/case_reduction.py#L88-L108: calculate source-buffer bytes separately from output-buffer bytes.
  • konfai/data/reduction.py#L214-L217: define Concat overhead against the revised byte model. Do not count the concatenated result twice.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@konfai/data/case_reduction.py` around lines 88 - 108, Update
konfai/data/case_reduction.py lines 88-108 in resident_regions, region_bytes,
and peak_bytes to model source-buffer bytes separately from output-buffer bytes,
rather than charging every resident region at the concatenated output size.
Update konfai/data/reduction.py lines 214-217 to define Concat’s working
overhead against this revised byte model and ensure the concatenated result is
counted only once.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.claude/skills/konfai-cli/SKILL.md:
- Around line 42-45: Update the TRANSFORM documentation in
.claude/skills/konfai-cli/SKILL.md at lines 42-45 to say --plan runs without
writing deliverable data and explicitly mention that it probes destinations,
potentially creating and removing an entry or creating a single-file H5 store.
Update examples/Transform/README.md at lines 50-53 to document that planning can
touch output locations and create an H5 file; remove wording that implies --plan
is fully read-only.

In @.github/workflows/konfai_apps_ci.yml:
- Line 47: Disable checkout credential persistence by adding
persist-credentials: false to the checkout steps in
.github/workflows/konfai_apps_ci.yml lines 47-47,
.github/workflows/konfai_ci.yml lines 38-38 and 92-94, and
.github/workflows/konfai_mcp_ci.yml lines 31-31. Keep fetch-depth: 0 unchanged
at .github/workflows/konfai_ci.yml lines 92-94.

In @.github/workflows/pre-commit.yml:
- Around line 16-22: Update the actions/checkout step in the pre-commit workflow
to set persist-credentials to false, ensuring the checkout token is not retained
before pre-commit/action runs; leave the existing checkout revision and
subsequent setup steps unchanged.

In @.github/workflows/publish.yml:
- Around line 280-284: Update the tags configuration in the publish workflow to
replace all three type=semver entries with type=pep440, preserving their
existing version, major.minor, and major patterns; leave the latest prerelease
condition unchanged.

In `@examples/Transform/make_dataset.py`:
- Around line 3-5: Update the module description and the status message around
the dataset-generation flow to state that cohort volumes have varying extents
rather than implying every volume has a unique extent. Preserve the existing
processing behavior and adjust only the affected descriptive text.
- Line 1: Add an Apache-2.0 SPDX-License-Identifier header at the beginning of
the Python source, before the module docstring.

In `@examples/Transform/README.md`:
- Around line 19-20: Update the setup section of the Transform README to add
python -m pip install "konfai[imaging]" before the first make_dataset.py example
command, ensuring the SimpleITK imaging dependency is installed before any
commands are run.

In `@konfai/data/patching.py`:
- Around line 2543-2548: Update materialize_copies() so the shared_pending sweep
loop stops immediately when _materialize_save(sweep) returns False, preventing
chained sweeps from overwriting _sweep_failure; preserve processing of
subsequent sweeps when each save succeeds.

In `@konfai/data/transform.py`:
- Around line 1215-1216: Ensure ResampleToReference honors self.interpolation in
the no-displacement-field path instead of relying solely on tensor.dtype. Update
the relevant _stream_mode() or resample_region() flow so explicit nearest and
linear modes select the requested sampler for all dtypes, and add regression
coverage for both no-field interpolation modes.

In `@konfai/main.py`:
- Around line 40-41: Update the TRANSFORM documentation to describe it as having
no top-level model while allowing nested KonfAIInference stages. In
konfai/main.py lines 40-41, replace “has no model” with “has no top-level
model”; in konfai/main.py lines 317-319 and .claude/skills/konfai-cli/SKILL.md
lines 42-45, replace wording that says TRANSFORM runs no model with equivalent
wording permitting nested inference stages.

In `@studio/konfai_studio/server.py`:
- Around line 894-899: Update the proxy-header guard in the request handling
flow around _trusts_proxy_headers() to reject requests when proxy trust is
disabled and any Forwarded, X-Forwarded-* (including supported variants), or
X-Real-IP header is present. Preserve the existing 403 response behavior, and
add regression coverage for each supported header form, including requests that
omit X-Forwarded-For.

---

Outside diff comments:
In `@konfai/data/case_reduction.py`:
- Around line 88-108: Update konfai/data/case_reduction.py lines 88-108 in
resident_regions, region_bytes, and peak_bytes to model source-buffer bytes
separately from output-buffer bytes, rather than charging every resident region
at the concatenated output size. Update konfai/data/reduction.py lines 214-217
to define Concat’s working overhead against this revised byte model and ensure
the concatenated result is counted only once.

In `@konfai/data/data_manager.py`:
- Around line 1979-1990: Update _output_destinations() so terminal Write/Save
transforms without an explicit dataset are included in validation by resolving
their destination through save_destination(), using the manager’s dataset
fallback consistently. Ensure the resulting resolved path and group are added to
destinations so _validate_write_chains() performs source-collision and
duplicate-target checks for Write: {} configurations.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7b82bcf6-751d-4165-b262-d0d4961074be

📥 Commits

Reviewing files that changed from the base of the PR and between 0952ed2 and 331a426.

📒 Files selected for processing (46)
  • .claude/skills/konfai-cli/SKILL.md
  • .github/workflows/commit-hygiene.yml
  • .github/workflows/konfai_apps_ci.yml
  • .github/workflows/konfai_ci.yml
  • .github/workflows/konfai_mcp_ci.yml
  • .github/workflows/konfai_studio_ci.yml
  • .github/workflows/pre-commit.yml
  • .github/workflows/publish.yml
  • CHANGELOG.md
  • docs/source/config_guide/transform.md
  • docs/source/development.md
  • docs/source/reference/api/extension-points.md
  • docs/source/reference/components/index.md
  • docs/source/reference/components/transforms.md
  • examples/README.md
  • examples/Synthesis/README.md
  • examples/Synthesis/Synthesis_demo.ipynb
  • examples/TotalSegmentator/TotalSegmentator_demo.ipynb
  • examples/Transform/README.md
  • examples/Transform/Transform.yml
  • examples/Transform/Transform_expand.yml
  • examples/Transform/make_dataset.py
  • examples/konfai_demo.py
  • konfai-mcp/konfai_mcp/guide.py
  • konfai-mcp/pyproject.toml
  • konfai/data/__init__.py
  • konfai/data/case_reduction.py
  • konfai/data/data_manager.py
  • konfai/data/patching.py
  • konfai/data/reduction.py
  • konfai/data/transform.py
  • konfai/main.py
  • konfai/predictor.py
  • konfai/transformer.py
  • konfai/utils/dataset.py
  • konfai/utils/ome_zarr.py
  • studio/konfai_studio/cli.py
  • studio/konfai_studio/server.py
  • studio/tests/test_quit.py
  • tests/integration/test_transform_doc_examples.py
  • tests/integration/test_transform_example.py
  • tests/unit/test_case_reduction.py
  • tests/unit/test_dataset.py
  • tests/unit/test_ome_zarr_data_surface.py
  • tests/unit/test_resample_sampler_rules.py
  • tests/unit/test_resample_to_reference.py

Comment thread .claude/skills/konfai-cli/SKILL.md Outdated
Comment thread .github/workflows/konfai_apps_ci.yml
Comment thread .github/workflows/pre-commit.yml
Comment thread .github/workflows/publish.yml
Comment thread examples/Transform/make_dataset.py
Comment thread examples/Transform/README.md
Comment thread konfai/data/patching.py
Comment thread konfai/data/transform.py
Comment thread konfai/main.py Outdated
Comment thread studio/konfai_studio/server.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
.github/workflows/konfai_apps_ci.yml (1)

47-47: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Sensitive Data Exposure (CWE-522): Insufficiently Protected Credentials

Reachability: External

Disable persisted checkout credentials in executable PR jobs.

Each listed checkout retains the GitHub token in local Git configuration. The job then executes pull-request-controlled installation, build, test, or hook code. That code can read and exfiltrate the token before checkout cleanup.

Add persist-credentials: false to each checkout step.

  • .github/workflows/konfai_apps_ci.yml#L47-L47: disable persistence before editable installation and app tests.
  • .github/workflows/konfai_ci.yml#L38-L38: disable persistence before editable installation and tests.
  • .github/workflows/konfai_ci.yml#L92-L94: disable persistence while retaining fetch-depth: 0.
  • .github/workflows/pre-commit.yml#L16-L16: disable persistence before pre-commit/action.
#!/bin/bash
set -euo pipefail

for file in \
  .github/workflows/konfai_apps_ci.yml \
  .github/workflows/konfai_ci.yml \
  .github/workflows/pre-commit.yml
do
  echo "== $file =="
  rg -n -C 5 \
    'pull_request:|permissions:|actions/checkout@|persist-credentials:|pip install -e|python -m build|pytest|pre-commit/action' \
    "$file"
done
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/konfai_apps_ci.yml at line 47, Disable persisted checkout
credentials by adding persist-credentials: false to the checkout steps at
.github/workflows/konfai_apps_ci.yml:47, .github/workflows/konfai_ci.yml:38,
.github/workflows/konfai_ci.yml:92-94, and .github/workflows/pre-commit.yml:16;
preserve fetch-depth: 0 at the applicable konfai_ci.yml checkout.

Source: Linters/SAST tools

🧹 Nitpick comments (5)
tests/unit/test_resample_sampler_rules.py (5)

25-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Align the module docstring with the covered paths.

This module tests Resample._resample_offset_region and compares it with SimpleITK. It does not invoke ResampleToReference._sample_at. Add that comparison or remove the claim that these tests compare both gather paths.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_resample_sampler_rules.py` around lines 25 - 27, Update the
module docstring in tests/unit/test_resample_sampler_rules.py to accurately
describe the covered paths: either add coverage comparing
ResampleToReference._sample_at with SimpleITK and the existing rules, or remove
the claim that the tests compare both gather implementations. Preserve the
existing Resample._resample_offset_region coverage.

78-87: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Cover non-zero patch coordinates.

Every current helper call uses _TARGET with zero starts, the full source tensor, and region_starts=[0, 0, 0]. This does not test the global-target/local-source contract. A patch-indexing regression can pass all tests. Add a case with non-zero target starts, a sliced source tensor, and matching non-zero region starts. Compare it with the whole-volume result.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_resample_sampler_rules.py` around lines 78 - 87, Extend the
resampling tests around _offset_region to cover non-zero patch coordinates: use
non-zero target_slices starts, a sliced local source tensor, and matching
non-zero region_starts while preserving the relevant scales and offsets. Compare
the patched result with the equivalent whole-volume _resample_offset_region
result to verify the global-target/local-source contract.

45-58: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Match _Sampler overrides to Resample.

Resample.transform_shape requires group_src, name, shape, and cache_attribute. _Sampler.transform_shape accepts only shape and cache_attribute. An inherited Resample.resample_source_region call can therefore raise TypeError. Match the base signatures and add annotations to the public overrides.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_resample_sampler_rules.py` around lines 45 - 58, Update the
_Sampler.transform_shape method signature to match the Resample.transform_shape
interface by adding the group_src and name parameters to the method definition.
Additionally, add type annotations to the public method overrides in _Sampler
(_Sampler.__call__, _Sampler.write_stream_cache_attribute,
_Sampler.transform_shape, _Sampler.inverse, and _Sampler.patch_locality) to
ensure the inherited resample_source_region call can correctly invoke
transform_shape with all required arguments without raising a TypeError.

Source: Coding guidelines


37-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keep the optional dependency skip local to the reference test.

Only test_the_separable_sampler_matches_simpleitk uses sitk. A missing SimpleITK installation currently skips the dtype, nearest-neighbor, and boundary tests too. Move the skip and import into that test or a fixture. Include the repository’s install hint in the skip reason.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_resample_sampler_rules.py` around lines 37 - 39, Move the
pytest.importorskip("SimpleITK") call and the import SimpleITK as sitk statement
from the module level into the test_the_separable_sampler_matches_simpleitk test
function (or a fixture it uses), so that only that specific test is skipped if
SimpleITK is missing. Include the repository's install hint in the skip reason
parameter passed to pytest.importorskip to help users understand how to install
the dependency.

Source: Coding guidelines


113-133: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add an exact half-way coordinate.

The current values do not produce a source coordinate exactly at k + 0.5. The test cannot distinguish round-half-up from another tie rule. Add a one-voxel case with scales=[1.0, 1.0, 1.0] and offsets=[0.5, 0.5, 0.5]. Assert that index 1 is selected on each axis.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_resample_sampler_rules.py` around lines 113 - 133, Add a
second test case within test_nearest_is_itk_round_half_up_and_not_a_size_ratio
that uses scales=[1.0, 1.0, 1.0] and offsets=[0.5, 0.5, 0.5] with a minimal
one-voxel source tensor to create coordinates that land exactly at k+0.5 (the
tie-breaking boundary). Update _SOURCE to [1, 1, 1] for this case, populate the
source tensor with a known value, then verify that index 1 is selected on each
axis when calling _offset_region and applying the coordinate formula with the
new scales and offsets. This ensures the test distinguishes round-half-up from
other tie-breaking rules by checking behavior at the exact half-way point.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@konfai-mcp/pyproject.toml`:
- Around line 17-22: Publish a konfai release newer than 1.7.0 before publishing
konfai-mcp, so the dependency constraint in the dependencies list is satisfiable
from PyPI. Keep the existing source-checkout behavior and constraint unchanged.

In `@konfai/data/patching.py`:
- Around line 2688-2693: Update the streamed-write planning around the slab
sizing logic near _SWEEP_ELEMENT_BYTES to enforce memory_budget using the landed
output shape, including channel expansion and wider dtypes, rather than only
source channels and four-byte elements. Validate a conservative output-byte
bound before opening the stream or validate the actual block size before
writing, and add coverage for channel-expanding stages and wider output dtypes.

In `@tests/unit/test_resample_sampler_rules.py`:
- Around line 94-97: Update the test fixture’s _volume offset from 1900.0 to
2050.0 so the entire generated range uses the claimed binary16 spacing of 2;
keep the surrounding explanation unchanged.

---

Duplicate comments:
In @.github/workflows/konfai_apps_ci.yml:
- Line 47: Disable persisted checkout credentials by adding persist-credentials:
false to the checkout steps at .github/workflows/konfai_apps_ci.yml:47,
.github/workflows/konfai_ci.yml:38, .github/workflows/konfai_ci.yml:92-94, and
.github/workflows/pre-commit.yml:16; preserve fetch-depth: 0 at the applicable
konfai_ci.yml checkout.

---

Nitpick comments:
In `@tests/unit/test_resample_sampler_rules.py`:
- Around line 25-27: Update the module docstring in
tests/unit/test_resample_sampler_rules.py to accurately describe the covered
paths: either add coverage comparing ResampleToReference._sample_at with
SimpleITK and the existing rules, or remove the claim that the tests compare
both gather implementations. Preserve the existing
Resample._resample_offset_region coverage.
- Around line 78-87: Extend the resampling tests around _offset_region to cover
non-zero patch coordinates: use non-zero target_slices starts, a sliced local
source tensor, and matching non-zero region_starts while preserving the relevant
scales and offsets. Compare the patched result with the equivalent whole-volume
_resample_offset_region result to verify the global-target/local-source
contract.
- Around line 45-58: Update the _Sampler.transform_shape method signature to
match the Resample.transform_shape interface by adding the group_src and name
parameters to the method definition. Additionally, add type annotations to the
public method overrides in _Sampler (_Sampler.__call__,
_Sampler.write_stream_cache_attribute, _Sampler.transform_shape,
_Sampler.inverse, and _Sampler.patch_locality) to ensure the inherited
resample_source_region call can correctly invoke transform_shape with all
required arguments without raising a TypeError.
- Around line 37-39: Move the pytest.importorskip("SimpleITK") call and the
import SimpleITK as sitk statement from the module level into the
test_the_separable_sampler_matches_simpleitk test function (or a fixture it
uses), so that only that specific test is skipped if SimpleITK is missing.
Include the repository's install hint in the skip reason parameter passed to
pytest.importorskip to help users understand how to install the dependency.
- Around line 113-133: Add a second test case within
test_nearest_is_itk_round_half_up_and_not_a_size_ratio that uses scales=[1.0,
1.0, 1.0] and offsets=[0.5, 0.5, 0.5] with a minimal one-voxel source tensor to
create coordinates that land exactly at k+0.5 (the tie-breaking boundary).
Update _SOURCE to [1, 1, 1] for this case, populate the source tensor with a
known value, then verify that index 1 is selected on each axis when calling
_offset_region and applying the coordinate formula with the new scales and
offsets. This ensures the test distinguishes round-half-up from other
tie-breaking rules by checking behavior at the exact half-way point.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 49494611-bb1b-4b54-81af-331074bde8f0

📥 Commits

Reviewing files that changed from the base of the PR and between 331a426 and 2b715b0.

📒 Files selected for processing (46)
  • .claude/skills/konfai-cli/SKILL.md
  • .github/workflows/commit-hygiene.yml
  • .github/workflows/konfai_apps_ci.yml
  • .github/workflows/konfai_ci.yml
  • .github/workflows/konfai_mcp_ci.yml
  • .github/workflows/konfai_studio_ci.yml
  • .github/workflows/pre-commit.yml
  • .github/workflows/publish.yml
  • CHANGELOG.md
  • docs/source/config_guide/transform.md
  • docs/source/development.md
  • docs/source/reference/api/extension-points.md
  • docs/source/reference/components/index.md
  • docs/source/reference/components/transforms.md
  • examples/README.md
  • examples/Synthesis/README.md
  • examples/Synthesis/Synthesis_demo.ipynb
  • examples/TotalSegmentator/TotalSegmentator_demo.ipynb
  • examples/Transform/README.md
  • examples/Transform/Transform.yml
  • examples/Transform/Transform_expand.yml
  • examples/Transform/make_dataset.py
  • examples/konfai_demo.py
  • konfai-mcp/konfai_mcp/guide.py
  • konfai-mcp/pyproject.toml
  • konfai/data/__init__.py
  • konfai/data/case_reduction.py
  • konfai/data/data_manager.py
  • konfai/data/patching.py
  • konfai/data/reduction.py
  • konfai/data/transform.py
  • konfai/main.py
  • konfai/predictor.py
  • konfai/transformer.py
  • konfai/utils/dataset.py
  • konfai/utils/ome_zarr.py
  • studio/konfai_studio/cli.py
  • studio/konfai_studio/server.py
  • studio/tests/test_quit.py
  • tests/integration/test_transform_doc_examples.py
  • tests/integration/test_transform_example.py
  • tests/unit/test_case_reduction.py
  • tests/unit/test_dataset.py
  • tests/unit/test_ome_zarr_data_surface.py
  • tests/unit/test_resample_sampler_rules.py
  • tests/unit/test_resample_to_reference.py
🚧 Files skipped from review as they are similar to previous changes (36)
  • examples/Synthesis/Synthesis_demo.ipynb
  • examples/Synthesis/README.md
  • .github/workflows/commit-hygiene.yml
  • tests/unit/test_case_reduction.py
  • tests/unit/test_ome_zarr_data_surface.py
  • examples/konfai_demo.py
  • studio/konfai_studio/cli.py
  • konfai/predictor.py
  • docs/source/reference/components/transforms.md
  • examples/README.md
  • .github/workflows/konfai_studio_ci.yml
  • docs/source/development.md
  • tests/integration/test_transform_doc_examples.py
  • konfai-mcp/konfai_mcp/guide.py
  • konfai/main.py
  • .claude/skills/konfai-cli/SKILL.md
  • tests/unit/test_dataset.py
  • studio/tests/test_quit.py
  • konfai/data/init.py
  • studio/konfai_studio/server.py
  • konfai/utils/ome_zarr.py
  • konfai/transformer.py
  • docs/source/reference/components/index.md
  • examples/TotalSegmentator/TotalSegmentator_demo.ipynb
  • examples/Transform/Transform_expand.yml
  • docs/source/reference/api/extension-points.md
  • konfai/utils/dataset.py
  • konfai/data/reduction.py
  • docs/source/config_guide/transform.md
  • examples/Transform/README.md
  • examples/Transform/Transform.yml
  • konfai/data/case_reduction.py
  • konfai/data/data_manager.py
  • tests/unit/test_resample_to_reference.py
  • konfai/data/transform.py
  • examples/Transform/make_dataset.py

Comment thread konfai-mcp/pyproject.toml
Comment thread konfai/data/patching.py
Comment thread tests/unit/test_resample_sampler_rules.py Outdated
@vboussot
vboussot force-pushed the fix/pre-1.8.0-review branch from 2b715b0 to 35244d6 Compare August 4, 2026 14:06

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unit/test_case_reduction.py`:
- Around line 395-408: Update the Vote implementation to compute modal labels
while explicitly resolving ties by selecting the smallest label, without relying
on torch.mode’s tie behavior. Preserve the input label dtype and add coverage in
test_vote_picks_a_label_where_median_would_invent_one for reversed input order
producing the same smallest-label result.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 69dd3bb9-518c-4b11-8395-394d4bae30b9

📥 Commits

Reviewing files that changed from the base of the PR and between 2b715b0 and 35244d6.

📒 Files selected for processing (46)
  • .claude/skills/konfai-cli/SKILL.md
  • .github/workflows/commit-hygiene.yml
  • .github/workflows/konfai_apps_ci.yml
  • .github/workflows/konfai_ci.yml
  • .github/workflows/konfai_mcp_ci.yml
  • .github/workflows/konfai_studio_ci.yml
  • .github/workflows/pre-commit.yml
  • .github/workflows/publish.yml
  • CHANGELOG.md
  • docs/source/config_guide/transform.md
  • docs/source/development.md
  • docs/source/reference/api/extension-points.md
  • docs/source/reference/components/index.md
  • docs/source/reference/components/transforms.md
  • examples/README.md
  • examples/Synthesis/README.md
  • examples/Synthesis/Synthesis_demo.ipynb
  • examples/TotalSegmentator/TotalSegmentator_demo.ipynb
  • examples/Transform/README.md
  • examples/Transform/Transform.yml
  • examples/Transform/Transform_expand.yml
  • examples/Transform/make_dataset.py
  • examples/konfai_demo.py
  • konfai-mcp/konfai_mcp/guide.py
  • konfai-mcp/pyproject.toml
  • konfai/data/__init__.py
  • konfai/data/case_reduction.py
  • konfai/data/data_manager.py
  • konfai/data/patching.py
  • konfai/data/reduction.py
  • konfai/data/transform.py
  • konfai/main.py
  • konfai/predictor.py
  • konfai/transformer.py
  • konfai/utils/dataset.py
  • konfai/utils/ome_zarr.py
  • studio/konfai_studio/cli.py
  • studio/konfai_studio/server.py
  • studio/tests/test_quit.py
  • tests/integration/test_transform_doc_examples.py
  • tests/integration/test_transform_example.py
  • tests/unit/test_case_reduction.py
  • tests/unit/test_dataset.py
  • tests/unit/test_ome_zarr_data_surface.py
  • tests/unit/test_resample_sampler_rules.py
  • tests/unit/test_resample_to_reference.py
🚧 Files skipped from review as they are similar to previous changes (39)
  • studio/konfai_studio/cli.py
  • konfai/utils/dataset.py
  • examples/Synthesis/Synthesis_demo.ipynb
  • examples/TotalSegmentator/TotalSegmentator_demo.ipynb
  • studio/tests/test_quit.py
  • .github/workflows/konfai_studio_ci.yml
  • tests/unit/test_dataset.py
  • .github/workflows/konfai_ci.yml
  • .github/workflows/pre-commit.yml
  • examples/konfai_demo.py
  • konfai/data/init.py
  • tests/unit/test_resample_sampler_rules.py
  • .claude/skills/konfai-cli/SKILL.md
  • examples/README.md
  • examples/Synthesis/README.md
  • docs/source/reference/api/extension-points.md
  • konfai-mcp/pyproject.toml
  • docs/source/reference/components/index.md
  • .github/workflows/konfai_mcp_ci.yml
  • examples/Transform/Transform.yml
  • studio/konfai_studio/server.py
  • konfai/data/data_manager.py
  • tests/unit/test_resample_to_reference.py
  • tests/unit/test_ome_zarr_data_surface.py
  • docs/source/development.md
  • konfai/main.py
  • konfai/utils/ome_zarr.py
  • examples/Transform/README.md
  • konfai/data/reduction.py
  • konfai/predictor.py
  • konfai/data/case_reduction.py
  • konfai/transformer.py
  • docs/source/config_guide/transform.md
  • .github/workflows/konfai_apps_ci.yml
  • tests/integration/test_transform_doc_examples.py
  • konfai/data/transform.py
  • examples/Transform/Transform_expand.yml
  • konfai-mcp/konfai_mcp/guide.py
  • docs/source/reference/components/transforms.md

Comment thread tests/unit/test_case_reduction.py
@vboussot
vboussot force-pushed the fix/pre-1.8.0-review branch from 35244d6 to c4cdae7 Compare August 4, 2026 14:15

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
konfai/data/patching.py (1)

2691-2704: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Enforce memory_budget for the landed slab.

If a streamed chain expands channels or produces a dtype wider than four bytes, _sweep_rows() underestimates the slab size. A successful streamed write returns before _enforce_fallback_budget() runs. The write can exceed the per-rank budget without refusal.

Derive an output-byte bound during planning and use it to size the slab. If no safe bound is available, refuse the streamed plan when a budget is set.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@konfai/data/patching.py` around lines 2691 - 2704, The _sweep_rows() method
currently calculates plane size using the SOURCE's channels and assumes a fixed
_SWEEP_ELEMENT_BYTES of four bytes, but the actual output may have expanded
channels or a wider dtype that is not known until after the slab is written.
Update _sweep_rows() to derive an output-byte bound during planning that
accounts for the actual output shape and dtype characteristics rather than
relying solely on source metrics. Use this output-byte bound to calculate plane
size accurately. If an output byte bound cannot be determined safely when a
memory budget is set, refuse the streamed plan instead of allowing an
underestimated slab size that could exceed the per-rank budget. Preserve the
existing fallback budget check logic that validates against real shapes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@konfai/data/patching.py`:
- Around line 2691-2704: The _sweep_rows() method currently calculates plane
size using the SOURCE's channels and assumes a fixed _SWEEP_ELEMENT_BYTES of
four bytes, but the actual output may have expanded channels or a wider dtype
that is not known until after the slab is written. Update _sweep_rows() to
derive an output-byte bound during planning that accounts for the actual output
shape and dtype characteristics rather than relying solely on source metrics.
Use this output-byte bound to calculate plane size accurately. If an output byte
bound cannot be determined safely when a memory budget is set, refuse the
streamed plan instead of allowing an underestimated slab size that could exceed
the per-rank budget. Preserve the existing fallback budget check logic that
validates against real shapes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: adb20825-9afb-46c4-a28f-39a924f0018b

📥 Commits

Reviewing files that changed from the base of the PR and between 35244d6 and c4cdae7.

📒 Files selected for processing (46)
  • .claude/skills/konfai-cli/SKILL.md
  • .github/workflows/commit-hygiene.yml
  • .github/workflows/konfai_apps_ci.yml
  • .github/workflows/konfai_ci.yml
  • .github/workflows/konfai_mcp_ci.yml
  • .github/workflows/konfai_studio_ci.yml
  • .github/workflows/pre-commit.yml
  • .github/workflows/publish.yml
  • CHANGELOG.md
  • docs/source/config_guide/transform.md
  • docs/source/development.md
  • docs/source/reference/api/extension-points.md
  • docs/source/reference/components/index.md
  • docs/source/reference/components/transforms.md
  • examples/README.md
  • examples/Synthesis/README.md
  • examples/Synthesis/Synthesis_demo.ipynb
  • examples/TotalSegmentator/TotalSegmentator_demo.ipynb
  • examples/Transform/README.md
  • examples/Transform/Transform.yml
  • examples/Transform/Transform_expand.yml
  • examples/Transform/make_dataset.py
  • examples/konfai_demo.py
  • konfai-mcp/konfai_mcp/guide.py
  • konfai-mcp/pyproject.toml
  • konfai/data/__init__.py
  • konfai/data/case_reduction.py
  • konfai/data/data_manager.py
  • konfai/data/patching.py
  • konfai/data/reduction.py
  • konfai/data/transform.py
  • konfai/main.py
  • konfai/predictor.py
  • konfai/transformer.py
  • konfai/utils/dataset.py
  • konfai/utils/ome_zarr.py
  • studio/konfai_studio/cli.py
  • studio/konfai_studio/server.py
  • studio/tests/test_quit.py
  • tests/integration/test_transform_doc_examples.py
  • tests/integration/test_transform_example.py
  • tests/unit/test_case_reduction.py
  • tests/unit/test_dataset.py
  • tests/unit/test_ome_zarr_data_surface.py
  • tests/unit/test_resample_sampler_rules.py
  • tests/unit/test_resample_to_reference.py
🚧 Files skipped from review as they are similar to previous changes (42)
  • examples/TotalSegmentator/TotalSegmentator_demo.ipynb
  • konfai/data/data_manager.py
  • konfai/main.py
  • .github/workflows/pre-commit.yml
  • examples/Transform/Transform.yml
  • konfai/predictor.py
  • konfai-mcp/konfai_mcp/guide.py
  • docs/source/development.md
  • examples/Synthesis/Synthesis_demo.ipynb
  • konfai/data/init.py
  • .github/workflows/konfai_mcp_ci.yml
  • docs/source/reference/components/index.md
  • konfai/transformer.py
  • studio/konfai_studio/cli.py
  • konfai-mcp/pyproject.toml
  • .github/workflows/konfai_apps_ci.yml
  • tests/unit/test_dataset.py
  • .github/workflows/konfai_ci.yml
  • examples/konfai_demo.py
  • examples/Transform/README.md
  • docs/source/reference/api/extension-points.md
  • studio/konfai_studio/server.py
  • tests/unit/test_resample_to_reference.py
  • konfai/data/case_reduction.py
  • tests/unit/test_ome_zarr_data_surface.py
  • tests/integration/test_transform_doc_examples.py
  • konfai/utils/ome_zarr.py
  • studio/tests/test_quit.py
  • examples/Transform/Transform_expand.yml
  • konfai/data/reduction.py
  • docs/source/config_guide/transform.md
  • examples/README.md
  • konfai/utils/dataset.py
  • examples/Transform/make_dataset.py
  • .github/workflows/konfai_studio_ci.yml
  • tests/unit/test_case_reduction.py
  • konfai/data/transform.py
  • examples/Synthesis/README.md
  • .github/workflows/commit-hygiene.yml
  • .claude/skills/konfai-cli/SKILL.md
  • docs/source/reference/components/transforms.md
  • tests/unit/test_resample_sampler_rules.py

@vboussot
vboussot force-pushed the fix/pre-1.8.0-review branch from c4cdae7 to 85872b1 Compare August 4, 2026 15:29

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
konfai/data/patching.py (1)

2691-2695: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Enforce memory_budget for landed streamed blocks.

Line 2691 sizes slabs from source channels and four-byte elements. A streamable one-hot or field-synthesis stage can produce more channels or a wider dtype. _materialize_save() and _materialize_shared_pass() then allocate the larger output block without calling _enforce_fallback_budget().

The fallback check runs only after streaming fails. A successful streamed write can therefore exceed memory_budget and fail with OOM.

Add an output channel and dtype resource contract. Use it to size or refuse slabs before the first output block allocation. Add coverage for channel expansion and wider output dtypes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@konfai/data/patching.py` around lines 2691 - 2695, The streamed slab sizing
around _materialize_save() and _materialize_shared_pass() assumes source
channels and four-byte elements, so expanded channels or wider output dtypes can
bypass memory_budget enforcement. Add an output channel/dtype resource contract
for streamable stages, resolve it before the first slab allocation, and use the
resulting output shape and item size to size slabs or reject streaming before
allocation; add coverage for channel expansion and wider output dtypes.
🧹 Nitpick comments (1)
tests/integration/test_transform_example.py (1)

41-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Pass workdir as a Path.

Line 41 converts workdir to str before passing it as cwd. Pass workdir directly to preserve the pathlib.Path API.

Proposed fix
-    return subprocess.run(command, capture_output=True, text=True, env=subprocess_env(), cwd=str(workdir), timeout=900)
+    return subprocess.run(command, capture_output=True, text=True, env=subprocess_env(), cwd=workdir, timeout=900)

As per coding guidelines, use pathlib.Path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/test_transform_example.py` at line 41, Update the
subprocess.run call in the test helper to pass workdir directly as cwd instead
of converting it with str(), preserving its pathlib.Path type and API.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unit/test_resample_sampler_rules.py`:
- Around line 37-39: Remove the module-level SimpleITK skip and import from
tests/unit/test_resample_sampler_rules.py. Add the availability check and
SimpleITK import inside test_the_separable_sampler_matches_simpleitk, leaving
the dtype, nearest-index, and boundary tests runnable without SimpleITK.

---

Duplicate comments:
In `@konfai/data/patching.py`:
- Around line 2691-2695: The streamed slab sizing around _materialize_save() and
_materialize_shared_pass() assumes source channels and four-byte elements, so
expanded channels or wider output dtypes can bypass memory_budget enforcement.
Add an output channel/dtype resource contract for streamable stages, resolve it
before the first slab allocation, and use the resulting output shape and item
size to size slabs or reject streaming before allocation; add coverage for
channel expansion and wider output dtypes.

---

Nitpick comments:
In `@tests/integration/test_transform_example.py`:
- Line 41: Update the subprocess.run call in the test helper to pass workdir
directly as cwd instead of converting it with str(), preserving its pathlib.Path
type and API.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 234b1475-b09d-4e20-b878-d5767517fa1c

📥 Commits

Reviewing files that changed from the base of the PR and between c4cdae7 and 85872b1.

📒 Files selected for processing (46)
  • .claude/skills/konfai-cli/SKILL.md
  • .github/workflows/commit-hygiene.yml
  • .github/workflows/konfai_apps_ci.yml
  • .github/workflows/konfai_ci.yml
  • .github/workflows/konfai_mcp_ci.yml
  • .github/workflows/konfai_studio_ci.yml
  • .github/workflows/pre-commit.yml
  • .github/workflows/publish.yml
  • CHANGELOG.md
  • docs/source/config_guide/transform.md
  • docs/source/development.md
  • docs/source/reference/api/extension-points.md
  • docs/source/reference/components/index.md
  • docs/source/reference/components/transforms.md
  • examples/README.md
  • examples/Synthesis/README.md
  • examples/Synthesis/Synthesis_demo.ipynb
  • examples/TotalSegmentator/TotalSegmentator_demo.ipynb
  • examples/Transform/README.md
  • examples/Transform/Transform.yml
  • examples/Transform/Transform_expand.yml
  • examples/Transform/make_dataset.py
  • examples/konfai_demo.py
  • konfai-mcp/konfai_mcp/guide.py
  • konfai-mcp/pyproject.toml
  • konfai/data/__init__.py
  • konfai/data/case_reduction.py
  • konfai/data/data_manager.py
  • konfai/data/patching.py
  • konfai/data/reduction.py
  • konfai/data/transform.py
  • konfai/main.py
  • konfai/predictor.py
  • konfai/transformer.py
  • konfai/utils/dataset.py
  • konfai/utils/ome_zarr.py
  • studio/konfai_studio/cli.py
  • studio/konfai_studio/server.py
  • studio/tests/test_quit.py
  • tests/integration/test_transform_doc_examples.py
  • tests/integration/test_transform_example.py
  • tests/unit/test_case_reduction.py
  • tests/unit/test_dataset.py
  • tests/unit/test_ome_zarr_data_surface.py
  • tests/unit/test_resample_sampler_rules.py
  • tests/unit/test_resample_to_reference.py
🚧 Files skipped from review as they are similar to previous changes (41)
  • .github/workflows/konfai_apps_ci.yml
  • examples/Synthesis/README.md
  • examples/Transform/Transform_expand.yml
  • docs/source/reference/api/extension-points.md
  • konfai/data/init.py
  • docs/source/reference/components/transforms.md
  • konfai/transformer.py
  • .github/workflows/commit-hygiene.yml
  • tests/unit/test_ome_zarr_data_surface.py
  • .github/workflows/konfai_studio_ci.yml
  • .github/workflows/konfai_mcp_ci.yml
  • tests/unit/test_resample_to_reference.py
  • .github/workflows/konfai_ci.yml
  • docs/source/reference/components/index.md
  • .github/workflows/pre-commit.yml
  • tests/unit/test_dataset.py
  • examples/README.md
  • examples/Transform/make_dataset.py
  • studio/konfai_studio/cli.py
  • studio/tests/test_quit.py
  • .claude/skills/konfai-cli/SKILL.md
  • studio/konfai_studio/server.py
  • tests/unit/test_case_reduction.py
  • konfai/utils/ome_zarr.py
  • konfai-mcp/pyproject.toml
  • konfai/predictor.py
  • examples/Synthesis/Synthesis_demo.ipynb
  • konfai-mcp/konfai_mcp/guide.py
  • konfai/main.py
  • examples/Transform/README.md
  • examples/Transform/Transform.yml
  • examples/TotalSegmentator/TotalSegmentator_demo.ipynb
  • docs/source/config_guide/transform.md
  • examples/konfai_demo.py
  • konfai/data/reduction.py
  • konfai/data/transform.py
  • konfai/data/case_reduction.py
  • docs/source/development.md
  • konfai/data/data_manager.py
  • konfai/utils/dataset.py
  • tests/integration/test_transform_doc_examples.py

Comment thread tests/unit/test_resample_sampler_rules.py Outdated
@vboussot
vboussot force-pushed the fix/pre-1.8.0-review branch 2 times, most recently from 9a1bcf6 to 324cbd4 Compare August 4, 2026 16:30

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
konfai/data/patching.py (1)

2690-2695: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Enforce memory_budget for the landed streamed slab.

Line 2691 sizes rows from source channels and four-byte elements. A successful streamed path returns from materialize() before _enforce_fallback_budget() runs. A channel-expanding stage or wider output dtype can therefore allocate an over-budget first slab and fail before fallback applies.

Derive a conservative output-byte bound during planning, or validate the actual landed slab before opening the stream. Do not treat the fallback guard as authority for successful streams.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@konfai/data/patching.py` around lines 2690 - 2695, Update the
streamed-materialization planning around materialize() and _SWEEP_ELEMENT_BYTES
to enforce memory_budget before opening the stream: derive a conservative bound
using the landed output channel count and dtype, or validate the actual first
slab before allocation. Ensure channel-expanding stages and wider dtypes are
covered, and do not rely on _enforce_fallback_budget() for successful streamed
paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/publish.yml:
- Around line 204-210: Replace the numeric-only version checks at both
classifier sites in .github/workflows/publish.yml (lines 204-210 and 237-245)
with one shared PEP 440-based classification mechanism. Ensure stable
post-releases such as v1.8.0.post1 are marked non-prerelease, while actual
prereleases remain prerelease, and reuse the same classifier for both GitHub
Release status and Docker latest tagging.

In `@docs/source/development.md`:
- Around line 241-251: Update the changelog paragraph in the release
documentation to state that the publish job fails when the tag’s section is
missing or empty, rather than implying only a missing section causes failure.
Keep the existing explanation about using the committed section verbatim for the
GitHub Release body.

---

Duplicate comments:
In `@konfai/data/patching.py`:
- Around line 2690-2695: Update the streamed-materialization planning around
materialize() and _SWEEP_ELEMENT_BYTES to enforce memory_budget before opening
the stream: derive a conservative bound using the landed output channel count
and dtype, or validate the actual first slab before allocation. Ensure
channel-expanding stages and wider dtypes are covered, and do not rely on
_enforce_fallback_budget() for successful streamed paths.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9d311257-fdf7-4a89-bbd0-127729c3ab3c

📥 Commits

Reviewing files that changed from the base of the PR and between c4cdae7 and 324cbd4.

📒 Files selected for processing (50)
  • .claude/skills/konfai-cli/SKILL.md
  • .github/scripts/release_notes.py
  • .github/workflows/commit-hygiene.yml
  • .github/workflows/konfai_apps_ci.yml
  • .github/workflows/konfai_ci.yml
  • .github/workflows/konfai_mcp_ci.yml
  • .github/workflows/konfai_studio_ci.yml
  • .github/workflows/pre-commit.yml
  • .github/workflows/publish.yml
  • CHANGELOG.md
  • docker/Dockerfile
  • docker/README.md
  • docs/source/config_guide/transform.md
  • docs/source/development.md
  • docs/source/reference/api/extension-points.md
  • docs/source/reference/components/index.md
  • docs/source/reference/components/transforms.md
  • examples/README.md
  • examples/Synthesis/README.md
  • examples/Synthesis/Synthesis_demo.ipynb
  • examples/TotalSegmentator/TotalSegmentator_demo.ipynb
  • examples/Transform/README.md
  • examples/Transform/Transform.yml
  • examples/Transform/Transform_expand.yml
  • examples/Transform/make_dataset.py
  • examples/konfai_demo.py
  • konfai-mcp/konfai_mcp/guide.py
  • konfai-mcp/pyproject.toml
  • konfai/data/__init__.py
  • konfai/data/case_reduction.py
  • konfai/data/data_manager.py
  • konfai/data/patching.py
  • konfai/data/reduction.py
  • konfai/data/transform.py
  • konfai/main.py
  • konfai/predictor.py
  • konfai/transformer.py
  • konfai/utils/dataset.py
  • konfai/utils/ome_zarr.py
  • studio/konfai_studio/cli.py
  • studio/konfai_studio/server.py
  • studio/tests/test_quit.py
  • tests/integration/test_transform_doc_examples.py
  • tests/integration/test_transform_example.py
  • tests/unit/test_case_reduction.py
  • tests/unit/test_dataset.py
  • tests/unit/test_ome_zarr_data_surface.py
  • tests/unit/test_release_notes.py
  • tests/unit/test_resample_sampler_rules.py
  • tests/unit/test_resample_to_reference.py
🚧 Files skipped from review as they are similar to previous changes (41)
  • docs/source/reference/components/index.md
  • .github/workflows/konfai_apps_ci.yml
  • docs/source/reference/components/transforms.md
  • examples/Synthesis/Synthesis_demo.ipynb
  • studio/konfai_studio/cli.py
  • .github/workflows/commit-hygiene.yml
  • examples/Transform/Transform.yml
  • examples/konfai_demo.py
  • konfai-mcp/konfai_mcp/guide.py
  • tests/unit/test_dataset.py
  • examples/README.md
  • konfai/data/init.py
  • examples/Synthesis/README.md
  • konfai-mcp/pyproject.toml
  • examples/TotalSegmentator/TotalSegmentator_demo.ipynb
  • tests/unit/test_resample_sampler_rules.py
  • tests/unit/test_case_reduction.py
  • konfai/predictor.py
  • studio/tests/test_quit.py
  • konfai/data/data_manager.py
  • .claude/skills/konfai-cli/SKILL.md
  • konfai/utils/dataset.py
  • tests/integration/test_transform_doc_examples.py
  • examples/Transform/README.md
  • tests/unit/test_ome_zarr_data_surface.py
  • konfai/main.py
  • .github/workflows/konfai_studio_ci.yml
  • .github/workflows/konfai_mcp_ci.yml
  • examples/Transform/make_dataset.py
  • docs/source/config_guide/transform.md
  • konfai/utils/ome_zarr.py
  • docs/source/reference/api/extension-points.md
  • konfai/transformer.py
  • tests/unit/test_resample_to_reference.py
  • konfai/data/reduction.py
  • .github/workflows/konfai_ci.yml
  • examples/Transform/Transform_expand.yml
  • studio/konfai_studio/server.py
  • .github/workflows/pre-commit.yml
  • konfai/data/case_reduction.py
  • konfai/data/transform.py

Comment thread .github/workflows/publish.yml Outdated
Comment thread docs/source/development.md
@vboussot
vboussot force-pushed the fix/pre-1.8.0-review branch 3 times, most recently from f4ef1b3 to bdc9999 Compare August 4, 2026 19:17
A uint8 label map resampled through a `field` was interpolated and truncated: `_stream_mode`
answered nearest for uint8, but only the two paths WITHOUT a field consulted it. Over a source
holding {0, 100} the warped path returned 29, 79 and 99 -- labels nobody wrote, in a volume that is
still a valid label map. ResampleToReference takes an `interpolation` now, and every sampler asks
one method for it: a declaration honoured on one path and ignored on another is worse than none,
because the page telling a user to set it is then right about half their chains.

A store chunked on the writer's region verbatim, and a slab sweep declares the whole trailing plane:
a gigabyte per chunk at 2048x2048, past what zarr holds in one buffer at 4096x4096, paid by every
partial read after. An axis the region covers end to end is tiled; the axis the writer advances
along keeps its height. Appending pyramid levels rewrote level 0 on ngff-zarr's default chunking, so
a streamed write and a declared pyramid cancelled each other out.

A statistic after a Reduce was seeded from the fold, so [Reduce, Clip, Normalize] normalised by the
unclipped Max and wrote a volume its own header did not describe. `grid: strict` skipped a geometry
key no header carried, which is quietest where it costs most -- a missing Direction is a flip that
shows in neither extent nor spacing. A second Reduce marker fell past the split and came back
diagnosed as an ordinary stage.

A reduction is budgeted at each side's own width: members at theirs, the output at its, and whatever
the operator builds over the buffer it was handed. Median stacks that buffer and sorts a copy, so a
cohort of N peaks at 3N+1 member regions where the plan said N+1; Concat writes N x C where each
member holds C, so charging the cohort at the output's width over-stated it by the cohort's size.
Vote is added beside them, because Mean and Median both answer with values that were in no input.

Warp built its grid on the CPU and met a GPU-resident volume in grid_sample; it also checks its
declared bound on the whole-volume path. Chained sweeps stop at the first failure -- on the shared
Expand pass as on the per-case one -- so the recorded reason is the cause and not a downstream
symptom. A refused plan no longer records the half-folded state it stopped at. _drawn_from restores
the CUDA generators it seeded. `from konfai.data import Clip` raised ImportError.

The three samplers stated one arithmetic at each site and had already drifted: the CPU-half guard
existed in three of them and not the fourth. sampling_dtype, nearest_index and window_index say each
rule once, over both gathers.
TRANSFORM shipped as the release's headline with no example, and the konfai-cli skill still
described three workflows mapping to three files. examples/Transform covers both directions the
cardinality can go: a median template folded from a cohort that shares no grid -- which is what
ResampleToReference is in the chain for -- and four drawn copies of every case.

The cohort is generated locally: six volumes, 3.5 MB, nothing downloaded, a minute on CPU. Written
uncompressed, because a compressed .mha cannot serve a disk region and the example would then
demonstrate the opposite of streaming. No two members share an extent, a spacing or an origin, so
`grid: strict` refuses them as stored and the resample in the chain is what makes it true.

Both configs are run AS SHIPPED by an integration test rather than templated into one, so a rename
in the grammar fails in CI and not in someone's terminal. The doc-examples test loses two ways of
quietly not running: the console script resolves through the harness helper that falls back to
`python -c` instead of skipping where the script is named differently, and a missing page is an
assertion rather than an empty parametrize set, which pytest reports as a pass.

The shared notebook helper asked for `--gpu 0`, which argparse rejects on a machine exporting
CUDA_VISIBLE_DEVICES=1, and dropped the last unterminated line from the tail it raises -- usually
the exception. Synthesis evaluates with SSIM and installed no scikit-image, so it failed after
training rather than before it. TotalSegmentator's notebook advertised an uncertainty step its
bundle does not build.
…annot be answered

/api/quit's loopback check reads the TCP peer, and behind the same-host reverse proxy REMOTE.md
documents, every peer IS 127.0.0.1. Uvicorn only rewrites it under --proxy-headers, which is off by
default and described as a logging concern, so the guard the docstring called sufficient was a no-op
for exactly the deployment the docs recommend. Any forwarding header now refuses without that flag
-- Forwarded, X-Real-IP or any X-Forwarded-*, because what matters is not which one arrived but that
one did -- and the shutdown task is held so the loop cannot collect it mid-sleep.

plan_transform told an agent it "writes no data" two sentences before saying it opens a real
region-write on each destination -- and those destinations are the user's own stores. It says so
where an agent reads it, along with the 50-entry cap on needs_attention.

konfai-mcp bounds konfai, because runner.py imports konfai.transformer at module scope and a 1.7.0
satisfies a bare name: the whole server then dies on import. Spelled `>1.7.0` and not `>=1.8.0` so a
source checkout satisfies it -- setuptools_scm builds the unreleased tree as 1.7.1.devN, which
PEP 440 sorts below 1.7.1.
The workflow re-rendered the release notes from the commits while the committed section is a draft
that was then edited -- squash merges collapse to one line, a subject with no conventional prefix is
dropped entirely, and a subject written for a reviewer says nothing to a user. So the file and the
release page were guaranteed to differ, which is the opposite of what publish.yml, development.md
and CHANGELOG.md all claimed. It takes the committed section verbatim now, and a tag whose section
is missing fails the job instead of publishing an empty release.

Every action is pinned to a commit SHA -- forty references across seven workflows, each carrying the
tag it resolved from so a reader can still tell what is pinned. There is a `contents: read` floor,
and no checkout persists the token: every one of these jobs installs and runs code a pull request
controls. konfai_mcp_ci fetches the tags, without which setuptools_scm builds the core package as
0.1.dev1 and no bound on it can hold.

Pre-release detection searched the whole tag for a letter, so `v1.9.0-backport` was a pre-release and
`latest` on Docker Hub moved for any tag at all; both now match the version against a numeric
pattern. The image's version tags move from semver to pep440, since `v1.8.0rc1` is a valid PEP 440
version and not a valid SemVer one -- semver rules match nothing there and the image would publish
with no version tag. Two CI path filters never fired on the changes they guard: the doc examples are
extracted and run from config_guide, and Studio's tests guard wiring that lives in konfai/.

The reference pages gain ResampleToReference and REGRID, which were documented at length in the
transform guide and absent from the tables an extension author reads, and the changelog gains a
"Behaviour changes" section: Median's value and dtype moved under a refactor line, and four new
refusals replace things that were being done silently and wrongly.

The image installs the wheels the run built instead of pulling them back from PyPI. That drops the
poll that waited up to five minutes for an index to serve what had just been uploaded, and it drops
ARG KONFAI_PYPI_VERSION -- the one version string committed anywhere, which did not follow the tag
and sat at 1.6.0 through all of 1.7. A local build now ships the working tree, and an image of a
published release is a `docker pull` of its tag rather than a rebuild.

The notes extraction moves out of the workflow into .github/scripts/release_notes.py, with a test.
It decides what a release publishes, runs once per tag in the job holding contents: write, and was
covered by nothing -- so its first version matched the tag with a `\b`, which also matches at a dot:
tagging `v1.8` would have published v1.8.0's notes under its own name, silently, because both are
real versions and the text reads fine.

The tag classifier follows PEP 440 instead of a numeric shape, and both jobs read it from the same
tested function: a numeric shape called the stable v1.8.0.post1 a pre-release, which would publish
it as one and hand `latest` to nothing.
@vboussot
vboussot force-pushed the fix/pre-1.8.0-review branch from bdc9999 to 59ac9a5 Compare August 4, 2026 19:21
@vboussot
vboussot merged commit a2a819c into main Aug 4, 2026
30 of 38 checks passed
@vboussot
vboussot deleted the fix/pre-1.8.0-review branch August 4, 2026 19:24
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.

1 participant