Audit LoRA projection coverage across model families - #49
Conversation
Broly Security ScanNote ✅ Clean scan Note Re-scan this PR anytime with
|
0458504 to
d1711e0
Compare
qywu
left a comment
There was a problem hiding this comment.
Two blocking issues, both mechanical.
1. This branch is not stacked on #47
git merge-base feature/qwen35-lora-projection-topology-20260811 feature/lora-all-model-projection-audit-20260811 resolves to d13788de, which is inside #41's branch, 3 commits past main. This branch forked from #41, not from #47, and re-implements #47's work as a separate commit (37dbab47 here vs a2e6b141 on #47 — different trees for the same qwen3_5* files).
As a result, 142 files present at #47's head are absent here, 20 of them non-test, including the whole DSV4-Flash lane that #45 delivers:
src/xorl/models/layers/moe/dsv4_native_combine.py
src/xorl/models/transformers/deepseek_v4/{exact_contract,exact_lm_head,native_payload}.py
src/xorl/ops/dsv4/exact_attention.py
src/xorl/ops/dsv4/kernel/tilelang_indexer{,_bwd}.py
src/xorl/models/transformers/glm5/exact_lora_contract.py
scripts/{capture,replay}_dsv4_exact_trace.py
scripts/{qualify_dsv4_marlin_lora,generate_dsv4_flash_exact_zero_adapter}.py
docs/k3/DSV4_FLASH_LORA_ZERO_K3_PLAN.md
git merge-tree against the declared base produces ~40 conflicts. Resolving them toward this branch would silently revert #43, #44, and #45. #46, #50, #51, and #52 all inherit this base, so the whole lower half of the stack is affected.
2. Undisclosed scope: the test consolidation
The first commit here, 527aafac "test: consolidate suite around production contracts", is 428 files, +34,672 / −39,549:
| base | this branch | |
|---|---|---|
| test functions | 2,195 | 283 (−87%) |
| test files | 353 | 236 |
src/ |
— | +128 / −4,681 |
It also removes production modules (src/xorl/rl/{__init__,kl,objectives,normalization}.py, ParallelRole.SAMPLER and ParallelPlan.glm52_sampler() from canonical_moe.py, canonical_moe_reduce_reference), rewrites the README and installation docs (Torch 2.10 → 2.12), and adds two working artifacts at the repo root — TEST_TRIAGE.md (5,516 lines) and test_audit_decisions.json (16,525 lines) — with a new check-added-large-files exclusion in .pre-commit-config.yaml so the JSON gets past the existing guard.
None of this is mentioned in the title or body, and the Validation section reads as though the PR is scoped to the LoRA audit.
The triage document is careful and the individual removals are argued, so this isn't a challenge to the work itself — but a few of the src/ deletions are circular (delete the test, the function becomes unreferenced, delete the function), and that reasoning deserves its own review rather than riding under a LoRA-audit title.
Requested changes
- Rebase onto #47 and drop the duplicate
37dbab47. - Split into two PRs: the LoRA projection audit, and the test consolidation with the deletion counts stated in the body. #46 is already named
cleanup-testsand its body says the consolidation moved to an ancestor — that seems like the intended home. - Move
TEST_TRIAGE.mdandtest_audit_decisions.jsonout of the repo root (or untrack the ledger) and drop thecheck-added-large-filesexclusion.
Worth noting the reason none of this was caught: .github/workflows/lint.yml triggers on pull_request: branches: [main], so it has not run on any PR in this stack except #41, and there is no test workflow at all.
|
Superseded by #57, which consolidates the complete exact trainer-serving stack directly onto main. |
Stacked on #47.
Summary
Validation