docs(reports): land cycles 14 and 16 with validation corrections - #277
Merged
Conversation
Lands three hardening reports, each revised after an independent validation pass against source rather than against the prior report. Cycle 16 + addendum (F1 — unreachable tree-sitter grammars): The finding survives. All measurements re-derived independently and confirmed: 46.2 MB unreachable vs 30.7 MB used, KotlinParser/CppParser unreferenced, and the counterintuitive result that only ccomment and preproc contain any C++ at all. Corrected: - Sequencing was backwards. preproc.rs::preprocess takes a &PreprocParser, and PreprocResults is a parameter of ParserTrait::new consumed only by the LANG::Cpp arm in parser.rs. Dropping preproc before C++ means gutting the macro-expansion path of a language still shipped. C++/Kotlin must go first; the preproc step is the most invasive, not the least, being the only one that reaches the crate's central trait. - Both file counts were inflated by substring matching -- language_cpp.rs carries ~40 C++ node names beginning "Preproc". True spread: 18 preproc, 14 ccomment, 19 union. - The C++ figure named the wrong quantity. The pair holds ~154 KB of C/C++; the C++ forcing musl-g++ is two scanner.cc files totalling 4,839 B. - The upstream-header quote was trimmed past its scoping clause and used as a prohibition it never stated; the macros.rs objection applied to both options, since the Mozjs excision edited macros.rs itself. - Grammar dependencies number nine, plus the tree-sitter core crate. Added: - Dropping preproc removes petgraph from the workspace entirely. preproc.rs is its only consumer, which retires the dependabot ignore rule and the roadmap bump item, and unblocks leiden-rs's petgraph feature -- disabled today solely to avoid conflicting with the pinned 0.6. - Semver: both steps remove public API from a published crate, so the next cut must be a minor bump. - Recurrence prevention: cargo-machete and cargo-shear do not apply. The grammars are referenced, just unreachable, and shear cannot see through mk_langs! without nightly --expand. Cycle 14 carries corrections from its own earlier validation pass. Cycle 15 is deliberately held back pending unapplied factual corrections and a ranking decision.
emrecdr
added a commit
that referenced
this pull request
Aug 17, 2026
* docs(reports): hardening cycle 17 — validating my own finding as shipped Anchor 9811bd8 (v0.28.0); baseline fbc9c93. Delta 7 commits (#277-#282) plus the cut. Cycle 16's F1 shipped as #278, a breaking change, so this cycle audits an implementation of the audit's own finding — and the corrections attached to my reports land harder than the finding did. EXCISION VALIDATED CLEAN. Five grammar crates remain; LANG has exactly six variants matching the six dispatched parsers; a word-boundary sweep for every removed identifier returns zero across .rs/.toml/.yml; preproc.rs and all four language_*.rs are gone; petgraph absent from Cargo.lock. Consumer coverage complete: CHANGELOG discloses the breaking API change, version 0.27.4 -> 0.28.0 is the correct semver-breaking bump for a published 0.x crate, and the musl roadmap row was rewritten rather than left stale. Payoff realised by construction — the surviving set is byte-identical to the set benchmarked at 25s, so the measured 33s of unreachable compile work is gone. The implementation went past the finding in four places, three of which are my gaps. (1) The four could NOT be separated: ParserTrait::new took Option<Arc<PreprocResults>>, consumed by exactly one arm (LANG::Cpp), so preproc was the MOST invasive of the four — my "two independently shippable steps, smallest first" sequencing was exactly backwards, and the pre-change signature was available to me. (2) petgraph and aho-corasick were also dead; my cycle-16 sweep covered codelore-lib and codelore-cli and skipped codelore-rca — the one crate the finding was about. (3) Downstream consumers I never traced: the dependabot ignore rule, the deferred 0.6->0.8 bump, and leiden-rs's petgraph feature, correctly left off with its comment reclassified. (4) ParserTrait::new simplified further, dropping a path argument used only by get_fake_code. CORRECTIONS ADJUDICATED — all verified against pre-change source. File counts inflated by substring matching (language_cpp.rs alone has 21 Preproc* node names; my method matched 19 files, 7 reference the module). The "~120 KB" named the wrong quantity — the C++ forcing musl-g++ is 4,839 B of scanner.cc. P3 asserted a property of eleven MCP tools while listing nine. The depth claim compared 57 analyses to a tool count. And the serious one: I quoted the crate header as "Don't refactor upstream code... keep the divergence from upstream minimal", eliding "to satisfy newer clippy lints" — turning a narrow instruction into a general prohibition, which I then used to reject the feature-gating option. The objection was also self-defeating, since the Mozjs excision I cited as precedent edited macros.rs itself. Worst error in seventeen cycles, because it was load-bearing for a recommendation rather than merely wrong. Rule added: quote whole when a quotation carries a recommendation, and check whether the objection also applies to the option being recommended. F (LOW, new) — the excision merged two roadmap rows into one problem. libduckdb-sys is now the ONLY C++-compiling dependency in the lockfile, so the "bundled-DuckDB compile dominator" (:119) and "re-add the musl target" (:121) share a single root cause. The three options at :119 are no longer interchangeable: sccache tuning does nothing for musl, and `dynamic` + pre-built DuckDB gives up the static linking that is the whole point of the musl target. Only build-once-and-cache closes both, and only if the cached artifact is built with a musl C++ toolchain. Recommend merging the rows and recording that option as load-bearing. Also validated: the CI concurrency fix is correct — the SHA is appended only on main, so each main commit gets its own group while PRs still cancel. * docs(reports): correct three claims cycle 17 got wrong Independent validation re-derived every structural claim from source, including at the pre-change commit. The six LANG variants, the zero-residual identifier sweep, the trait signature, the 21/7 counts in section 2 and the verbatim CHANGELOG quotation all hold. Three claims did not. - Section 1.2 said petgraph and aho-corasick were "both now absent from Cargo.lock". Only petgraph left. aho-corasick is still in the lockfile, pulled by globset, regex and regex-automata; dropping it from codelore-rca's manifest removed an unused direct dependency, not the crate from the build, so its compile cost is unchanged. Written without opening the lockfile -- and written in the paragraph claiming credit for finding dead dependencies, two sections before adjudicating this exact defect class in earlier reports. - Section 3 called libduckdb-sys the only C++-compiling dependency. iana-time-zone-haiku also ships a scanner, but it sits behind cfg(target_os = "haiku") and compiles on no target this project builds for. The enumeration was incomplete; the finding it supports is unaffected. - Section 6 described gh-pages as "13 behind". It is an orphan branch with no common ancestor with main, so "behind" does not measure anything there. The 21 and 7 counts were checked against both a literal and a natural reading before being accepted: 21 is the distinct node types once the numbered nesting variants collapse (36 raw), and 7 is the files depending on the preproc module's exports (6 via crate::preproc, plus macros.rs via PreprocResults). --------- Co-authored-by: Emre <emre@valocom.nl>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lands three hardening reports, each revised after an independent validation pass run against source rather than against the prior report.
F1 survives, better bounded
Re-derived independently and confirmed: 46.2 MB of unreachable generated grammar source against 30.7 MB actually used;
KotlinParser/CppParserunreferenced from first-party code (verified non-vacuously against aRustParsercontrol); and the counterintuitive result that onlyccommentandpreproccontain any C++ at all — mozcpp and kotlin-ng are pure C.Corrected
The recommended sequencing was backwards.
preproc.rs::preprocesstakes a&PreprocParser, andPreprocResultsis a parameter ofParserTrait::new(traits.rs:54) consumed only by theLANG::Cpparm (parser.rs:76-84). Dropping preproc first means gutting the macro-expansion path of a language still being shipped. C++/Kotlin must go first; the preproc step is the most invasive of the two, being the only one that reaches the crate’s central trait.Both file counts were inflated by substring matching —
language_cpp.rscarries ~40 C++ node names beginningPreproc. True spread: 18 preproc, 14 ccomment, 19 union.The C++ figure named the wrong quantity. The pair holds ~154 KB of C/C++; the C++ that actually forces
musl-g++is twoscanner.ccfiles totalling 4,839 B.The upstream-header quote was trimmed past its scoping clause ("to satisfy newer clippy lints") and used as a prohibition it never stated. The
macros.rsobjection applied to both options anyway — the Mozjs excision editedmacros.rsitself (UPSTREAM.md:49).Grammar dependencies number nine, plus the
tree-sittercore crate.Added
Dropping
preprocremovespetgraphfrom the workspace.preproc.rsis its only consumer;codelore-libmerely documents that it avoids it. This retires the dependabot ignore rule and the roadmap bump item, and unblocks leiden-rs’spetgraphfeature — disabled today solely to avoid conflicting with the pinned 0.6 (codelore-lib/Cargo.toml:59-63). This is the strongest argument for that step and no earlier pass found it.Semver: both steps remove public API from a crate published at 0.27.4, so the next cut must be a minor bump.
Recurrence prevention:
cargo-macheteandcargo-sheardo not apply — the grammars are referenced, just unreachable, and shear cannot see throughmk_langs!without nightly--expand. The project’s own guard-test convention is the right mechanism, added after the excision.Scope
Docs only; no code touched. Cycle 15 is deliberately held back pending unapplied factual corrections and a ranking decision.
No excision has been performed — the corrected plan awaits a decision on sequencing and the semver bump.