Skip to content

feat: add learned partial mirror planner - #536

Merged
JustVugg merged 2 commits into
JustVugg:devfrom
bherald:contrib/partial-mirror-planner
Jul 31, 2026
Merged

feat: add learned partial mirror planner#536
JustVugg merged 2 commits into
JustVugg:devfrom
bherald:contrib/partial-mirror-planner

Conversation

@bherald

@bherald bherald commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a learned partial-mirror planner driven by .coli_usage
  • inspect safetensor headers directly and retain gate-projection dependencies for selected expert shards
  • enforce copy budget and free-space reserve, reject unsafe/duplicate paths and malformed metadata, and fail closed without usage evidence
  • stage copies atomically, fsync, verify SHA-256, and publish a readiness receipt without moving or deleting authoritative shards

Why

Very large disk-streamed MoE models may span multiple volumes. Mirroring the empirically hottest complete shard/dependency set onto a faster or less contended volume can be tested safely without changing the authoritative model layout.

Validation

  • complete native suite plus 150 Python tests passed, 10 skipped, after rebasing onto JustVugg/colibri:dev revision cd278c227cf515b1f123c5fcc6bcf3c4820eafc9
  • focused planner tests cover learned ranking, direct safetensor metadata, dependency inclusion, missing-usage refusal, traversal rejection, copy, receipt readiness, and full hash verification
  • read-only real-model plan observed 1,369,800 selections and admitted 71 shards / 188,562,803,696 bytes under a 200-GiB budget while preserving a 100-GiB reserve

No model files were copied for the real-model validation; planning alone never activates a mirror.

@JustVugg

Copy link
Copy Markdown
Owner

Thanks — a learned mirror planner for dual-SSD is a sensible tool, and it's cleanly isolated (Python mirror_plan.py + launcher + test, no engine core touched), so it's low-risk. But it's +636 lines of a feature that isn't on the current critical path (the focus is getting the large models running on a single box), and I haven't given it the deep review that size deserves. So I'm parking it as not-priority-now rather than merging on a surface pass — it stays open and mergeable. When the dual-SSD path (#421) is something we're actively tuning, this is the natural companion. No rush on your side.

@JustVugg JustVugg added the enhancement New feature or request label Jul 28, 2026
The only conflict was the handler dict in c/coli: this branch adds
"mirror":cmd_mirror, dev added "tune":cmd_tune in JustVugg#673. Both are real
commands with their own cmd_ function, so both entries are kept -- picking a
side would have silently deleted a working subcommand.

Verified: coli --help renders, and both 'coli mirror' and 'coli tune' resolve.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JustVugg
JustVugg merged commit 09c4d96 into JustVugg:dev Jul 31, 2026
9 of 10 checks passed
@JustVugg

Copy link
Copy Markdown
Owner

Merged — and since I told you the opposite ten days ago, you are owed the reasoning rather than just a state change.

What I said then was:

"+636 lines of a feature that isn't on the current critical path … I haven't given it the deep review that size deserves. So I'm parking it as not-priority-now rather than merging on a surface pass."

The blocker there was my review time, not a defect in your work. I have now done that review, and it is the reason the verdict changed:

  • Nothing in the engine. Four files: README.md, c/coli, c/tests/test_mirror_plan.py, c/tools/mirror_plan.py. No colibri.c, no quant.h, no st.h, no backend.
  • The launcher change is purely additive. A new cmd_mirror, a new mirror subparser, and one line in the handler dict. The single deleted line is that dict entry, replaced by the same entry plus yours. coli chat, run, serve and web are byte-for-byte unchanged, and nothing runs unless someone types coli mirror.
  • stage cannot destroy data. The only unlink calls are on its own temporary file during copy_verified, which copies and checks a sha256 against the plan. No rmtree, no move, no truncation of anything in the source model. That was the one thing that could have made "isolated Python" not enough, and it holds.
  • Its 8 tests pass.

One resolution I made on your behalf, because I would rather you hear it from me than find it in the log. Your branch conflicted with dev in exactly one place: the handler dict, where #673 had added "tune":cmd_tune. Picking either side would have silently deleted a working subcommand, so I kept both and verified that coli mirror and coli tune each resolve. Nothing else of yours was touched and no commit was rewritten.

The companion note still stands: when the dual-SSD path (#421) is something we are actively tuning, this is the tool that makes a partial mirror worth planning rather than guessing. It is in dev now, so it will be there when that happens.

Thank you for the patience, and for shipping it with a test suite — that is most of why the review was quick once I actually sat down to it.

terrizoaguimor pushed a commit to terrizoaguimor/colibri that referenced this pull request Aug 1, 2026
dev went red on macOS only, in the test added by JustVugg#536:

  AssertionError: PosixPath('/private/var/folders/.../same.safetensors')
              != PosixPath('/var/folders/.../same.safetensors')

On macOS /var is a symlink to /private/var, so tempfile hands back
/var/folders/... while mirror_plan.py resolves every path it is given
(discover_shards line 115, create_plan lines 197-199). The test compared a
resolved path returned by the tool against an unresolved one it built itself.
Linux has no such symlink, which is why it passed there -- including in my own
pre-merge run, which is how this reached dev.

Resolving the temp root in setUp makes every derived path resolved, so both
sides match on every platform. Test-only; mirror_plan.py is unchanged.

Verified by reproducing the macOS condition on Linux with a symlinked temp dir:
the old comparison fails against it and the new one passes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants