feat: integrate lean-md as an external lean-ctx addon (+ LSP formatter routing) - #721
Conversation
yvgude
left a comment
There was a problem hiding this comment.
Thorough first-pass review while this is still draft — overall this is exactly the shape we asked for: the reverse-cut discipline is real (both sides of the invariant gated: outbound ctx_* surface must survive, ctx_md_render/ctx_md_check must be gone), the .lmd.md raw-read e2e test is properly hermetic (private LEAN_CTX_DATA_DIR + --fresh), the registry migration cleanly retires the lmd placeholder, and the headless symbols_overview fallback is a genuine improvement beyond the addon itself. CI is green across the board (Windows job still pending as I write this).
Three points need fixing before ready-for-review, plus a few nits.
Blockers
B1 — Unrelated upstream docs are deleted. The PR removes six maintainer planning/spec documents that have nothing to do with lean-md:
docs/superpowers/plans/2026-06-17-dependency-auto-update-ci.mddocs/superpowers/specs/2026-06-17-dependency-auto-update-ci-design.mddocs/superpowers/specs/2026-06-17-dependency-upgrades-plan-a-design.mddocs/superpowers/specs/2026-06-17-dependency-upgrades-plan-b-design.mddocs/superpowers/specs/2026-06-19-prose-model-spike.mddocs/superpowers/specs/2026-06-19-token-company-competitive-improvements-design.md
Looks like an artifact of the reverse-cut script or a merge resolution. Please restore them (git checkout upstream/main -- docs/superpowers/).
B2 — Directory-scope reformat regresses to "unchanged" + stale caches. The new handle_reformat_refactor computes blake3_of(&abs_path) before/after and invalidates only that one path. Two problems:
- For
scope=Directory,abs_pathis a directory —std::fs::read(dir)errors,.ok()yieldsNoneon both sides,before == after→ the tool reports "unchanged" even when the IDE reformatted files, and no cache entry is invalidated. - The old
render_reformatiteratedres.changed_pathsfrom the JetBrains backend and invalidated every changed file;run_jetbrains_reformatnow discards them (.map(|_| ())). Even for file scope, anoptimize_importspass can touch more than the target.
Suggestion: keep the BLAKE3 changed/unchanged honesty for the Formatter::Command path (single file, it's a real improvement), but let the JetBrains arm return changed_paths and invalidate them all, reporting changed files: N as before.
B3 — nextest.toml: dropping test-threads = -4 is out of scope. The comment right above it explains why parallelism is capped (shared-state flakiness). If the cap genuinely blocks your suite, make that case in its own PR — silently changing global test parallelism inside an addon-integration PR is how flaky CI comes back.
Nits
docs/reference/21-lean-md.md§5 linksdocs/lean-md/specs/2026-06-25-lmd-v2-addon-decoupling-design.md, which doesn't exist upstream and isn't added by this PR — dead link; point it at the addon repo instead.ctx_read.rshunk is whitespace-only inside atracing::debug!string continuation (semantically neutral, but it's rustfmt drift — please drop it).reverse_cut_gate.rsTest 1's doc header still listsctx_read.rsas the "auto-render delegation hook" survivor — stale now that the delegation was itself reverse-cut and reads are raw.- The tiktoken warmup in
cache.rs::hebbian_eviction_bonus_is_wiredis well-reasoned and well-documented, but it's an unrelated test-hardening ride-along; fine to keep, just flagging it for the changelog.
On the design itself
The RenderTransform trait is appropriately minimal, and capabilities.exec: ["lean-ctx"] in the registry entry matches the outbound-contract story in §3.3 of the new doc. min_lean_ctx: "3.8.12" is correct for the manifest/registry features you rely on. Once B1–B3 land I'm happy to take this out of draft — the skills-as-phased-render payload is a compelling first real consumer of the addon surface.
|
Follow-up on the distribution question (how does the What it means for this PR, concretely:
So: no side-loading hacks needed, and crates.io is the bridge, not the destination. |
|
Update on the distribution story discussed above: the infrastructure for prebuilt binaries just landed as PR #729 (Phase 1 of #724). Concretely for lean-md, once that merges you can add an [artifacts.aarch64-apple-darwin]
filename = "lean-md-aarch64-apple-darwin"
url = "https://github.com/dasTholo/lean-md/releases/download/v0.2.0/lean-md-aarch64-apple-darwin"
sha256 = "…"
# one entry per platform you build in CI
|
|
Phase 2 of #724 just merged to # once, on your machine
lean-ctx account login && lean-ctx account claim dastholo
# from the lean-md repo (next to lean-ctx-addon.toml)
lean-ctx addon publish --namespace dastholo --check # all gates offline, no upload
lean-ctx addon publish --namespace dastholo # sign + upload to ctxpkg.comAfter that, anyone can run So for this PR that means: no waiting on a curated-registry MR round-trip — you can ship |
|
oh a lot to do! I have to take a closer look! I'll sit down right away. But it will probably only become something on Wednesday |
yvgude
left a comment
There was a problem hiding this comment.
Review — PR #721: lean-md addon integration
Thank you for this thorough contribution! The reverse-cut approach is well-reasoned and the PR is cleanly structured. A few items before merge:
1. CLA ⚠️
The CLA checkbox is unchecked — please sign the Contributor License Agreement by checking the box in the PR description. We cannot merge without it.
2. Technical feedback
Positive:
- The reverse-cut gate tests (`reverse_cut_gate.rs`) are a great pattern — they prevent engine symbols from leaking back into the core binary. Would love to see this as a reusable CI template.
- The `RenderTransform` trait in `extension_registry.rs` is well-designed as a generic hook point.
- The `.lmd.md` raw-read decision (no auto-render delegation) is the right call — it keeps `ctx_read` simple and deterministic.
Requests:
- `lsp/format/mod.rs` (254 lines): Please split the formatter resolver (`resolve_formatter_for_extension`) and the command runner (`run_formatter_command`) into separate submodules. Our LOC gate enforces ≤1500 lines for new files, so this is fine now, but the module is doing two distinct things.
- Tests for the formatter routing: The `tests_ops.rs` additions cover the `ctx_refactor` surface, but there are no unit tests for the new `format/mod.rs` itself (edge cases: missing `rustfmt`, fallback to IDE, BLAKE3 unchanged detection). Please add a few.
- Doc cleanup: `21-lean-md.md` references `WasmRenderTransform` in the decoupling table — this was removed in the later commits but the doc still mentions it. Please verify the doc matches the final code state.
3. Merge conflicts
The PR was last rebased against v3.9.3 `main`. We've since merged several proxy and config changes. Please rebase onto current `main` and verify the conflict resolution in `ops.rs` and `integration_tests.rs` is still clean.
Summary: Great contribution, well-motivated. Please:
- ✅ Sign the CLA
- 🔄 Rebase onto current `main`
- 📝 Add formatter unit tests + fix doc reference
Looking forward to landing this!
|
@dasTholo — friendly follow-up on the review feedback from July 8th. We've merged 4 PRs into
The architecture is solid and we'd love to land this. Let us know if you need any help with the rebase or have questions about the feedback. |
Adds RenderTransform trait (name/render with hint) + render_transforms registry map and register/lookup/names methods on ExtensionRegistry. Generic @render type=<name> infra; .lmd.md consumer wired via gateway in a later task.
…e (N1 link, N3 header)
…changed, invalidate all changed paths (B2)
…hs (B2 regression guard)
The very first count_tokens() in a process lazily loads the tiktoken BPE tables, which can exceed the 500ms co-access burst window. Without a warmup the two store() calls straddle that window and never associate, leaving the eviction bonus flaky-empty. Warm up once so both reads stay in one burst. Test-only; no production behavior change.
…existent WasmRenderTransform
ffca08c to
03b6413
Compare
|
All contributors have signed the CLA. ✅ |
The appendix pointed at the bundled slug, which is listed-only and refuses addon add. The raw-read gate only asserted a plain marker, which survives a rendered read; asserting on the unconsumed @Date directive is what makes it bite.
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
|
CLA Assist doesn't like me anymore |
|
@dasTholo — welcome aboard with the CLA signed! 🎉 Since you last rebased, 3 PRs have been merged to Quick rebase stepsgit fetch upstream main
git rebase upstream/mainExpected conflict areas (based on the merged diffs):
None of these should conflict with your actual Remaining review items (from the earlier feedback)
Let me know if you need help with the rebase — happy to do it on your behalf if time is tight. |
rust-1.97.0 clippy flags the inline Vec<(&str, fn(&str) -> Result<Snapshot, String>)> as type_complexity under -D warnings, which breaks the example "gen_registry" build in CI. Extract the fn-ptr into a Canonicalize type alias, exactly as clippy suggests.
…survivors The merge with main pulled in the yvgude#727/yvgude#780 addon-dependency work, whose test fixtures reference the lean-md addon by name/slug: - rust/src/cli/addon_deps.rs (self-dependency guard tests: "@dasTholo/lean-md") - rust/src/core/addons/pack_env.rs ({pack_dir:} expander tests: "@dasTholo/lean-md-skills") These are addon-name/slug references in #[test] code, exactly the survivor category the gate already allows (never in-tree render ENGINE code). Add both to the no_lmd_symbols_outside_docs_and_hook allowlist and the survivor doc list (seven -> nine).
|
Brought the branch up to current main (v3.9.7) via merge. GitHub reports the PR as MERGEABLE — the lean-md changes have zero file overlap with recent main churn, so the conflict areas anticipated earlier (ops.rs, Two CI failures surfaced purely from the merge with current main, both now fixed:
On the review items from your last note:
|
lean-md cut v0.2.0 (5 real SHA-256 in lean-ctx-addon.toml). Lift its curated registry entry to the same 3.9.6 contract and stamp the release version. verified stays false (registry authority).
feat: integrate lean-md as an external lean-ctx addon (+ LSP formatter routing)
Summary
Integrate
lean-md— a directive/macro Markdownrenderer for agent plans — into lean-ctx as an external addon, not in-tree code. The
renderer engine (developed in-tree across phases 1–9) is reverse-cut before merge: it
never reaches
main. What lands here is only the thin surface lean-ctx needs to host andreach the addon.
Why lean-md. Agents re-describe the same plan scaffolding every turn. lean-md gives them
token-lean, cache-safe, reusable plans —
@define/@callmacros,@if/@consumerphase-isolation,
@renderpipes — single-sourced across a plan. Its code-intel directives(
@read,@refactor,@search, …) call back into lean-ctx over the wire through the stablectx_*surface: lean-md brings the plan grammar, lean-ctx brings the code intelligence.Why an addon (not in-tree, not a fork). The renderer is standalone (
rushdown+evalexpr) with no lean-ctx crate dependency, and lean-ctx ships no render-engine dependencyin return. Keeping it out of tree means an independent release cycle, no coupling of the core
binary to the render engine, and a small, auditable contract — and it reuses the addon
ecosystem that matured in 3.8.x instead of forking/recompiling lean-ctx.
What this PR actually changes (10 commits, 12 files):
.lmd.mdis read raw byctx_read— like any other file. There is no.lmd.mdspecial-casing anywhere in
ctx_read.rs; rendering is the addon's job, reached explicitlythrough its
ctx_md_render/ctx_md_checktools once installed. A gate test(
rust/tests/ctx_read_lmd_md_raw.rs) pins this.addon_registry.json: thelmdplaceholder becomes thelean-mdentry, and stayslisted (
command: "", no[install]block ⇒is_installable() == false). It exists foraddon searchdiscovery; installation goes through the hosted pack(
addon add @dasTholo/lean-md). Three registry tests move to the new slug with it.extension_registry::RenderTransformtrait + registry — infra for@render type=<name>, not lmd-exclusive.ctx_refactor/ LSP improvement rides along: extension-based reformat routing(
.rs→rustfmt, else IDE) with honest BLAKE3 change detection, and cache invalidation ofall changed paths.
rust/tests/reverse_cut_gate.rs) fail the build if any enginesymbol/dep/seed-dir leaks back into
rust/src.ctx_read.rsis deliberately not excludedfrom the symbol scan — after the cut it carries no lmd knowledge, so the gate scans it like any
other file.
This branch was rebuilt from
upstream/mainand force-pushed. It no longer deletes anydocs/superpowers/*file and no longer touchesrust/.config/nextest.toml— both wereartefacts of the previous branch history and are gone. It is also file-disjoint from #780
(the #727 follow-up fixes), so the two can land in either order.
Test plan
cd rust && cargo test— run ascargo nextest run(this repo's runner): 8762 passed,21 skipped, 0 failed
cd rust && cargo clippy --all-targets --all-features -- -D warnings— cleancd rust && cargo fmt --check— cleannpm test/ build steps — n/a, none changedTargeted gates:
cargo nextest run --test reverse_cut_gate— no engine symbols inrust/src, no render-enginedeps, seed dir removed, the
ctx_*outbound surface survives,ctx_md_render/ctx_md_checkabsent from the core
cargo nextest run --test ctx_read_lmd_md_raw— a.lmd.mdread returns raw sourcecargo nextest run -E 'test(/registry/)'— 199 passed, includingbundled_registry_passes_security_validator,bundled_addon_registry_is_canonicalandevery_runnable_bundled_addon_declares_scrubbing_capabilities(which skips listed entries)The live-addon roundtrip (
addon add @dasTholo/lean-md→ctx_tools list→ctx_md_render) isnot exercised in CI — no addon is installed there. It depends on the distribution follow-up
listed below.
Notes for reviewers
rust/src/tools/registered/ctx_read.rsis byte-identical toupstream/mainon thisbranch (
git diff upstream/main -- rust/src/tools/registered/ctx_read.rsis empty). Theearlier auto-render delegation hook was never re-applied. Zero risk there.
rust/src/lsp/format/mod.rs+rust/src/tools/ctx_refactor/ops.rs— the reformat path nowresolves a formatter by extension and reports a real
changed/unchangedvia before/afterBLAKE3 instead of assuming a live IDE. It invalidates the cache for every changed path, not
just the first;
tests_ops.rsguards that regression.rust/data/addon_registry.jsonis a generated file (bundled_addon_registry_is_canonicalenforces canonical form). The
lmd→lean-mdrename shifted the entry's alphabetical sortposition, so the generator re-sorted it:
lean-mdnow sorts beforeletta. The diff showsthat swap; both entries are listed with identical empty
[mcp]blocks, so nothing wasexchanged between them.
lmdslug. The soleremaining
"lmd"string in the tree is anis_slugliteral test incore/addons/manifest.rs.min_lean_ctxon the entry is3.9.4; that is a metadata statement about the addon, not agate on this PR.
[capabilities]requirement is imposed on existing manifests.docs/reference/21-lean-md.md(new) — addon integration reference: raw read, listed entry,the
ctx_*callback surface, and the decoupling rationaledocs/reference/appendix-mcp-tools.md— records that.lmd.mdis returned raw and thatctx_md_render/ctx_md_checkare addon-providedOpen follow-ups (not in this PR)
lean-mdandswitching the entry to a real install path. This PR intentionally lands the integration surface
only, and says so honestly in the entry (
listed, notinstallable).rust/src/lsp/format/mod.rsfurther, if you want it smaller than it landed.lean-md::prefix).lean-ctx call.Contributor License Agreement