perf: load conditional-path libs on demand instead of inlining every branch - #217
Conversation
Agent Skills environments (Grok Build, Codex, Antigravity) have no runtime `!cat`, so the transformer inlined every referenced lib into SKILL.md. That put all four reviewer BACKEND loops — copilot, github, local-agent, ollama — into each of the seven commands that run a review, ~130KB (~33K tokens) of which a run uses at most one. `/do:better-swift` reached 414KB, and the whole skill set 2.8MB. Those four are now written into the skill's own `lib/` directory and cited by a relative path with an imperative read directive; the dispatcher (`multi-reviewer-loop.md`) stays inline, since it is always on the taken path and is what names the backend to load. Skill set drops to 1.82MB (-35%); per-command 32-52% on everything that reviews. Supporting changes: - A bundled child is transformed against the parent's present-set, so it cites the dispatcher it was split from rather than re-appending it. Without this each backend file came out larger than its source and the split saved nothing at read time. - The backticked see-also form (`lib/x.md`) now resolves too, since that token denotes a real bundled file once bundling exists. It renames only — routing it through the appendix queue pulled whole docs into skills that merely name-drop them, inflating /do:config from 19KB to 91KB. - Uninstall removes the bundle directory rather than stranding it beside a deleted SKILL.md. Claude Code and OpenCode keep runtime `!cat` and never reach this path; their output is byte-identical, verified against all 21 installed command files. Content reachability verified across 8657 substantive lines: nothing dropped.
Two places in next.md state the same thing twice, which is drift risk more
than it is bulk:
- The Phase 1 pre-flight bash comment restated the whole GH_HOST derivation
("seed with $ORIGIN_HOST, then apply the snippet's fallbacks and per-host
auth precheck") that the prose immediately above the `!cat` states again.
The comment now keeps only what is unique to it — why `gh api` needs an
explicit --hostname — and defers the rest to the one authoritative spot.
- The deletions-win conflict rule appeared in Phase 5 and again in Phase 6,
and the two had already drifted: only Phase 6 carried the load-bearing ban
on `git add -A` while paths are unmerged. Phase 5 is now the complete
statement, including that ban, and Phase 6 refers to it.
Deliberate repetition elsewhere in this file is left alone. The four
DEFAULT_BRANCH derivations and the two jq probes are separate shell
invocations that cannot share variables — the file says so at each site —
and the Parse Arguments flag reference legitimately restates what the
execution steps do.
…ow out Extends the bundling mechanism from the four reviewer backends to every lib that sits on a branch a run may not take, and splits the one large conditional section that lived inline rather than in lib/. Newly deferred, each verified conditional at its include site: - plan-issue-mode (18.9K x6) and epic-children (8.9K x2) — issues mode only; PLAN.md mode, the default, never opens the tracker. - The six review lenses (46K) — review-agent-selection dispatches only the lenses a diff signals, often one or two, sometimes none. - enhance-loop (23.7K) — its heading already reads "only when --enhance-with". - ci-flake-handling (3.4K x2) — reached only when a check fails like a flake. - next-swarm (32.8K, new) — /do:next's --swarm flow, extracted from next.md. A single-issue run, the default, no longer carries the parallel-batch flow. Deliberately NOT deferred: code-review-checklist sits under a REQUIRED GATE, and Phase 1 of /do:better-swift says to load swift-gotchas "into your context" before launching agents. Both are always on the taken path, where deferring buys an extra read and risks the agent skipping content it always needed. That rule is now written down beside the list. DEFERRED_LIBS becomes ON_DEMAND_LIBS, a map carrying `when` (the branch that makes the read required) and `what` (the content) per entry. Both render into the directive, so the agent is told when it must read a file instead of being left to infer it. Skill set 2.80MB -> 1.55MB (-45%, ~314K tokens); /do:review -56%, /do:plan-task -84%, /do:rpr -60%. No skill grew. Two contract suites read commands/do/next.md directly and would have lost sight of the swarm rules once they moved. Their readers now resolve `!cat` includes, so they scan the composed document the agent actually sees. Claude/OpenCode keep runtime `!cat`; next-swarm.md installs into ~/.claude/lib like any other lib and composes back to the same content.
3fd36ad to
a800882
Compare
atomantic
changed the title
atomantic
left a comment
atomantic
left a comment
There was a problem hiding this comment.
Reviewed by /do:review — 1 critical, 2 improvements.
Codex runtime smoke: PASS on a normal install. At this exact head, Codex CLI 0.149.1 discovered a PR-generated Agent Skill, followed the generated lib/next-swarm.md required-read directive, read the sibling bundle, and returned the expected sentinel. The full suite also passes 424/424, and a generated 21-skill Codex tree had no dangling bundled references.
Overall verdict: not clean yet. The new uninstall path follows a symlinked bundle directory; an isolated reproduction deleted a file outside the skill tree and then failed with ENOTDIR. The clean Codex runtime works, but this data-loss path should be fixed before merging.
Coherence check
The PR description's statement that all 21 Claude/OpenCode installed command files are byte-identical is not literal after the next-swarm extraction: do/next.md / do-next.md changes to a runtime include. The composed behavior may be equivalent, but the claim should say composed output/behavior rather than installed command-file bytes.
Generated by /do:review
|
Codex runtime passes on |
|
Codex runtime also passes on the final bundle-health commit |
…e multi-reviewer-loop docs
atomantic
left a comment
atomantic
left a comment
There was a problem hiding this comment.
Review Summary for PR #217
Verdict: Clean — verified full compatibility across all supported environments including Antigravity CLI (agy), Codex, Grok, Claude Code, and OpenCode. All 428 tests passing.
Highlights & Validation
- Antigravity CLI Compatibility:
- Agent skills are properly structured with
SKILL.mdandlib/<filename>.mdbundles in~/.gemini/antigravity-cli/skills/<skill-name>/. - On-demand lib directives generate clear instructions with the exact bundled location so Antigravity and other subagents can read them without path confusion.
- Zero dangling
~/.claude/lib/links or broken relative references across all 21 commands and bundled files.
- Agent skills are properly structured with
- Security & Invariants:
- Symlink traversal protection (
assertSafeBundlePath) properly blocks file/directory symlinks during install, update, and uninstall. - List health checks correctly report missing, modified, or stale bundled libraries.
- Symlink traversal protection (
- Fixes Applied & Pushed:
- Updated
lib/multi-reviewer-loop.mdline 62 to reflect on-demand bundled library resolution for Agent Skills hosts (Antigravity/Codex/Grok). - Enhanced
src/installer.jsdoUninstallto prune empty skill directories when removing directory-namespaced skills, and added test verification intest/installer.test.js.
- Updated
Generated by /do:review
1 participant
Agent Skills environments (Grok Build, Codex, Antigravity) have no runtime
!cat, so the transformer inlined every referenced lib intoSKILL.md— putting every branch of a command into every install, whether or not a run takes it./do:better-swifthad reached 414KB; the whole skill set 2.8MB.Libs on a conditional path are now written into the skill's own
lib/directory and cited with a read directive naming the branch that makes them required.Result
~314K tokens. No skill grew — the eleven not listed are byte-identical.
What is deferred, and why each qualifies
local-agent-review-loopcodex/claude/agy/grok/cursorollama-review-loopollamagithub-reviewer-loop@<login>copilot-review-loopcopilotplan-issue-modeepic-childrenenhance-loop--enhance-withpassedci-flake-handlingnext-swarm(new)--swarmpassedThe four reviewer backends are mutually exclusive — one
--review-withentry dispatches exactly one. The dispatcher (multi-reviewer-loop.md) deliberately stays inline: it is always on the taken path and is what names which backend to load.next-swarm.mdis new —/do:next's--swarmflow, extracted fromnext.md. A single-issue run, the default, no longer carries the parallel-batch flow.What is deliberately NOT deferred
code-review-checklist.mdsits under a REQUIRED GATE, and/do:better-swiftPhase 1 says to loadswift-gotchas.md"into your context" before launching agents. Both are always on the taken path, where deferring buys an extra read and risks the agent skipping content it always needed. That rule is written down beside the list, so the next entry gets held to it.Also in this PR
/do:nexthad two genuinely restated instructions — the Phase 1 pre-flight comment repeated the whole GH_HOST derivation the prose already gives, and the deletions-win conflict rule appeared in both Phase 5 and Phase 6 and had already drifted (only Phase 6 carried the ban ongit add -Awhile paths are unmerged). Phase 5 is now complete; Phase 6 refers to it.Deliberate repetition elsewhere is untouched: the four
DEFAULT_BRANCHderivations and twojqprobes are separate shell invocations that cannot share variables, and the file says so at each site.Verification
!catand never reach this path. Verified byte-identical across all 21 installed command files;next-swarm.mdinstalls into~/.claude/liblike any other lib and composes back to the same content.SKILL.md+ its bundle. The six that differ are this PR's own intended rewrites.lib/<name>.mdcited in a SKILL.md resolves to an installed file.--listand--dry-runboth clean.Two bugs found and fixed while building this
/do:configfrom 19KB to 91KB. It now renames only.Both have regression tests.