Keep _modified when collapsing a plan for a re-plan

[d-chambers]

Description

Fixes #871.

_modified on a plan member says the member is a trim of its source rather than the whole of it, and PlanResolver._load_member reads it as exactly that: a member without it loads its source with no trim at all.

collapse_working_df dropped the column one line before the re-plan read it, and _build_members falls back to "no source was modified" when it is absent. Every collapsed row was a trim, so the fallback asserted the opposite of the truth, the unchanged test below it was then satisfied trivially — a member's range always equals its own range — and a 500-sample slice of a 2000-sample file came back marked "load whole".

Keeping the column is the whole fix.

patch = dc.get_example_patch()                       # 8 s, 2000 samples
first = dc.spool(patch).chunk(time=2)
first._catalog.resolver.member_rows["_modified"]     # [True, True, True, True]
second = first.chunk(time=3)
second._catalog.resolver.member_rows["_modified"]    # [False, True, True, False]  <- before
sum(p.shape[1] for p in second)                      # 4500, from a 2000-sample patch

The else np.zeros(...) fallback in _build_members stays: build_chunk_plan is also called on plain relations which never had the column. It was only wrong for a frame which had one and lost it.

What changes

• This issue's case: the re-chunked selection held 252 samples of which 201 were distinct; it now holds the 201 the selection holds, all distinct.
• chunk(time=2).chunk(time=3): 4500 samples → 1500, which is what chunk(time=3) gives directly (two full 3-second chunks; the trailing 2 s is partial and dropped by default). The two paths now yield byte-identical data.
• chunk(distance=100).chunk(distance=None): raised CoordMergeError — three members each reloading the full 300-channel patch, handed to an assembler asked to vary them along distance — and now rebuilds the original patch exactly.

No existing test needed updating.

Note for #883

#883 (phase 3c of the inventory work) met this from the other side: channel-level select builds a plan whose pieces deliberately do not cover their source, so collapsing one loaded back the channels it had removed. It guards against that with a lossy flag which refuses to collapse such a plan.

With this fix applied and that guard disabled, select(coupling="trench").chunk(distance=None) already loads the correct channels — so the guard is belt-and-braces rather than the fix. It is worth keeping either way ("a plan which dropped data must not be re-derived from the data it dropped" holds however the bookkeeping is spelled), and the two do not conflict: a lossy plan simply never reaches the collapse this corrects.

Changelog

• fixed: re-planning a dimension after a chunk loaded whole source patches instead of the trimmed pieces the first plan described, so chunk(...).chunk(...) and a re-chunked selection over-included and duplicated samples (Re-chunking a selected chunked spool ignores the select, duplicating samples #871).

Checklist

I have:

• filled in the Changelog section above (see docs/contributing/general_guidelines.qmd).

I have (if applicable):

• referenced the GitHub issue this PR closes.
• documented the new feature with docstrings and/or appropriate doc page.
• included tests. See testing guidelines.
• added the "ready_for_review" tag once the PR is ready to be reviewed.

Summary by CodeRabbit

• Bug Fixes

  • Preserved trim status when preparing indexed data for reprocessing.
  • Prevented trimmed data from being incorrectly reloaded as complete source patches.
  • Improved consistency when rechunking nested or repeated data selections.
  • Ensured selected coordinate ranges are retained during rechunking.

• Tests

  • Added regression coverage for trimmed data handling and patch reconstruction.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4bbfd61a-21cd-43ef-8ad6-49f206377cd5

📥 Commits

Reviewing files that changed from the base of the PR and between dc155f4 and b5022eb.

📒 Files selected for processing (2)

• dascore/io/index/planned.py
• tests/test_io/test_index/test_planned.py

---

📝 Walkthrough

Walkthrough

The change preserves _modified trim flags during catalog collapse. Regression tests cover nested chunking, selected-range re-chunking, duplicate prevention, and reconstruction from trimmed chunks.

Changes

Planned catalog trim state

Layer / File(s)	Summary
Preserve _modified during collapse dascore/io/index/planned.py	collapse_working_df removes output_id while retaining _modified, so downstream loading reapplies source trims.
Validate re-planning and reconstruction tests/test_io/test_index/test_planned.py	Tests cover trim-flag retention, nested chunking, selected-range re-chunking, duplicate prevention, and reconstruction from trimmed chunks.

Suggested labels: bug, patch, spool

Mergeability Score: ⚪ Minimal · up to b5022

The change preserves trim metadata during re-planning and adds focused regression coverage; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the primary fix: preserving _modified during plan collapse for re-planning.
Description check	✅ Passed	The description explains the issue, implementation, affected behaviors, changelog entry, linked issue, and regression tests.
Linked Issues check	✅ Passed	The changes directly address issue #871 by preserving trim metadata and preventing duplicated or over-included samples during re-chunking.
Out of Scope Changes check	✅ Passed	The implementation and regression tests remain within the scope of issue #871 and the stated re-planning objective.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-collapse-drops-modified

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (ae85d31) to head (b5022eb).
⚠️ Report is 2 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##               dev      #886    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          177       177            
  Lines        20797     21077   +280     
==========================================
+ Hits         20797     21077   +280     

Flag	Coverage Δ
network	45.94% <0.00%> (-0.40%)	⬇️
unittests	100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[d-chambers]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.