docs(reports): land cycle 17 with three claims corrected - #283
Merged
Conversation
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.
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).
This was referenced Aug 17, 2026
emrecdr
added a commit
that referenced
this pull request
Aug 18, 2026
…nding (#285) * docs(reports): hardening cycle 18 — the sweep run properly, and one correction rejected Anchor 19a00ec; baseline 9811bd8 (v0.28.0). Delta 2 commits (#283, #284), both consequences of cycle 17. #284 VALIDATED. The roadmap merge implements cycle 17's finding F faithfully: one row, the three options ranked rather than listed, and build-once-and-cache marked as the only one closing both outcomes. It also absorbs the correction #283 made to that same finding — the row says libduckdb-sys is the only C++-compiling dependency "that builds on any target we ship", repairing my flat claim rather than leaving the fix in a report. CORRECTIONS: two accepted, one rejected. Accepted — aho-corasick is still in Cargo.lock (pulled by globset, regex, regex-automata). Cycle 17 said petgraph and aho-corasick were "both now absent"; only petgraph left. Removing it from codelore-rca's manifest deleted an unused DIRECT dependency, not the crate from the build. #283's framing is fair: written without opening the lockfile, in the paragraph claiming credit for finding dead dependencies. Accepted — iana-time-zone-haiku also ships a scanner, so calling libduckdb-sys "the only C++-compiling dependency" was incomplete; it sits behind cfg(target_os = "haiku") and builds on no target we ship, so the finding stands. REJECTED, with evidence — "gh-pages is an orphan branch, so 'behind' does not measure anything there." The orphan fact is correct and new to me (git merge-base main gh-pages is empty). The conclusion is not: git branch -v's [behind N] never compares to main, it compares a branch to its own upstream. gh-pages tracks origin/gh-pages, and `git rev-list --count gh-pages..origin/gh-pages` is 13 — exactly what cycle 17 §6 said ("13 behind locally, which is the publishing job"), a phrasing that already encodes cycle 9's E1 lesson. Recorded as rejected rather than quietly accepted, because deferring to a correction without verifying it is the same failure as accepting a finding without verifying it. The underlying observation — that this figure invites misreading, twice now — stands. F (LOW, new) — codelore-rca declares three more dependencies it does not use: serde_json, num-traits, rayon. Zero occurrences anywhere in its src/, no aliased imports that could hide them; the same vendoring residue class as petgraph and aho-corasick. Found by finally running the sweep properly — all three crates, all dependency tables, all source roots including build.rs, with - to _ normalisation. Cycle 16's version covered two crates with a crude method and skipped the one the finding was about. Bounded before claiming, per the aho-corasick lesson: removing all three changes the build by NOTHING. serde_json is a direct dep of both lib and cli; rayon of lib; num-traits is pulled by the whole arrow stack, chrono, criterion and the num-* family. This is manifest hygiene only — the value is that a vendored crate's manifest overstates what the vendored code needs, which misleads whoever is working out what the fork still depends on. Complementary to #278's note that unused-dependency tooling would not have found the grammars: it would find these three. The classes are disjoint — unreferenced-declared (tooling catches) vs referenced-but-unreachable (reachability analysis catches) — and the project has now hit one of each. cargo-machete is the natural guard for the first. * docs(reports): land cycle 18, withdraw a wrong correction, fix its finding Two corrections in opposite directions, both settled by running a command rather than by reading text. Cycle 17's gh-pages correction is WITHDRAWN. It claimed "13 behind" was meaningless because gh-pages is an orphan branch with no common ancestor with main. The orphan fact is true and irrelevant: [behind N] never compares to main, it compares a branch to its own upstream. gh-pages tracks origin/gh-pages, `git rev-list --count gh-pages..origin/gh-pages` is 13, and `git branch -vv` prints "[origin/gh-pages: behind 13]" verbatim. The original figure was correct and standard. The correction measured gh-pages..main (807) -- a comparison the report never made -- and generalised from it. Cycle 18 section 2.3 rejected it with that evidence; the rejection is upheld and the retraction lands here. Cycle 18's own finding is CORRECTED. It listed three unused dependencies in codelore-rca. Only two are removable: serde_json + rayon removed -> cargo check --all-targets: 0 errors num-traits removed -> error[E0463]: can't find crate for `num_traits` in six language_*.rs files codelore-rca derives FromPrimitive in six generated files, and num-derive's documentation states its macros assume num_traits is a direct dependency unless the #[num_traits = "..."] helper names another path -- which this crate does not use. The derive emits bare num_traits:: paths that must resolve in this crate's own extern prelude, so a transitive copy does not help. Three claims fall with it: "all three unreferenced", the "removing all three changes the build by nothing" bound, and the tooling note's assertion that cargo-machete would find all three. It would flag num-traits as a false positive, so the recommendation now carries the qualifier that any such step must ignore num-traits and be read as a candidate list. The finding survives as two removable dependencies, and gains a better lesson than it started with: "absent from the source text" and "safe to remove" are different predicates, and only the second is checkable by building. * docs(reports): make cycle 17's header note match its withdrawn correction The header still listed the gh-pages figure among the corrections made in place. That correction is withdrawn in the same PR, so the summary now counts two surviving corrections and points at the retraction. --------- 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.
Cycle 17 audits the implementation of cycle 16's own finding (#278, the breaking grammar excision) and adjudicates the corrections attached to #277 and #282. Landed after an independent validation pass.
What held
Every structural claim re-derived from source, including at the pre-change commit
6382f11^:LANGhas exactly six variants.rs/.toml/.ymlParserTrait::new(code);metrics_with_guardkeepspathpetgraphabsent from lockfileoutputSchemaat 1 of 11 MCP toolsThe section-1 footnote is real and independently reproducible: lowercasing
DocCommentMarkergenuinely containsccomment, which is what produced the eleven phantom residual hits it describes catching.Two counts I initially suspected turned out correct, and were checked before being challenged:
PreprocElif,PreprocElif2,PreprocElif3,PreprocElif4) collapse to one node type.crate::preproc/mod preproc, plusmacros.rsviaPreprocResults= 7 dependents.What was corrected
Section 1.2 — the aho-corasick claim. The report said
petgraphandaho-corasickwere "both now absent fromCargo.lock". Onlypetgraphleft.aho-corasickremains, pulled byglobset,regexandregex-automata— dropping it fromcodelore-rca's manifest removed an unused direct dependency, not the crate from the build. Its compile cost is unchanged. Written without opening the lockfile, in the paragraph claiming credit for finding dead dependencies.Section 3 — the C++ enumeration.
iana-time-zone-haikualso ships a C++ scanner, but behindcfg(target_os = "haiku"), so it compiles on no target this project builds for. Enumeration incomplete; the finding it supports is unaffected.Section 6 — the gh-pages figure. Described as "13 behind"; it is an orphan branch with no common ancestor with
main, so "behind" measures nothing there (raw counts: 805 and 168).On the new finding
F (LOW) holds and is a genuine second-order observation: removing the grammars merged two previously independent roadmap rows into one decision, which retroactively changed the option ranking in a row the change never touched. Dynamic linking plus a pre-built DuckDB speeds the build but defeats the static binary that motivates the musl target; only build-once-and-cache closes both.
Docs only; no code touched.