test(analyses): scan the whole file for the bot-filter collapse

CHANGELOG.md

@@ -6,6 +6,8 @@ Conventional Commits format. All notable changes documented here.
 
 ### Changed
 
+- **The bot-filter guard now reads the whole file, not one line at a time.** The previous fix taught it that SQL is case-insensitive and whitespace-tolerant, but it still normalised each line independently — so a call wrapped across lines was never assembled and its argument never seen. That is the house style in the directory it scans: a long `SUM(` there already puts its argument and closing paren on their own lines, and a `BOOL_OR(` that grew as long would be written the same way. The file is now normalised once and matches are mapped back to a real file line by counting newlines in the prefix, the technique the SPA escaping guard already uses. Verified by planting wrapped SQL, which the previous matcher passed and this one fails naming the construct's own line, and by confirming all four single-line spellings are still caught.
+
 - **`zizmor` now audits the workflows on every pull request.** It is the standard static analyser for GitHub Actions, and running it against this repository found three live template-injection sites that nothing here would have caught — which is the argument for adopting it. Exactly one audit is configured, and configured to agree with a policy this repository already enforces: full commit SHAs for third-party actions, tags permitted for GitHub's own namespace and for the toolchain action whose tag names the Rust version rather than a release. Two gates disagreeing about pinning would be worse than either alone. That alignment drops the report from 111 findings to 74, and from 44 high-severity to 7, without suppressing any of them. The gate blocks, because the seven findings it reported were resolved rather than deferred: the release pipeline's workflow-level write permission was reduced to read — the three jobs inheriting it write nothing here, and the one job that creates the release already declared its own — while the Dependabot auto-merge workflow's two triggers and three write scopes, and one low-confidence cache finding, carry written exceptions on the lines that raise them. An advisory version was written first and discarded on seeing it: a check that is red on every pull request teaches people to ignore red checks, which is worse than not running the tool.
 
 - **The bot-filter guard now matches the query's meaning rather than one spelling of it.** It forbids collapsing bot-ness to a single value per canonical identity outside the shared alias-resolving CTE — a canonical mixing human and bot aliases is silently misclassified by that collapse, which moves the author and ownership numbers other analyses read. It looked for two exact literals, case-sensitively. SQL is case-insensitive and indifferent to whitespace, so the lowercase form, a spaced form, and a table-qualified column are the same query and the same misclassification, and all three were invisible to it. Matching now runs against a normalised copy with the qualifier stripped. The tree was clean either way — the only occurrences are the documented exemption in the CTE's own explanatory comment — so nothing was misclassified; what was missing was detection. Verified by planting a comment that is lowercase, spaced and qualified at once and confirming the guard names its file and line where the previous matcher saw nothing.

crates/codelore-lib/tests/bot_filter_hygiene_test.rs

@@ -60,16 +60,16 @@ fn no_canonical_level_bool_or_is_bot_outside_query_rs() {
             continue;
         }
         let text = std::fs::read_to_string(file).expect("read source file");
-        for (line_idx, line) in text.lines().enumerate() {
-            if collapses_bot_per_canonical(line) {
-                let rel = file.strip_prefix(&root).unwrap_or(file);
-                violations.push(format!(
-                    "{}:{}: {}",
-                    rel.display(),
-                    line_idx + 1,
-                    line.trim()
-                ));
-            }
+        for at in collapse_offsets(&text) {
+            let rel = file.strip_prefix(&root).unwrap_or(file);
+            let line_idx = text[..at].matches('\n').count();
+            let line = text[at..].lines().next().unwrap_or_default();
+            violations.push(format!(
+                "{}:{}: {}",
+                rel.display(),
+                line_idx + 1,
+                line.trim()
+            ));
         }
     }
 
@@ -83,6 +83,53 @@ fn no_canonical_level_bool_or_is_bot_outside_query_rs() {
     );
 }
 
+/// Byte offsets in `text` where a per-canonical collapse begins.
+///
+/// The whole file is normalised once rather than line by line. SQL here is
+/// written to a house style that wraps a long call across lines — see the
+/// `SUM(` in `analyses/ownership.rs`, whose argument and closing paren sit on
+/// their own lines — so a `BOOL_OR(` whose argument grew long enough to wrap
+/// would be formatted the same way and never assembled by a per-line scan.
+/// Matching what the planner sees means ignoring layout as well as spelling.
+///
+/// Offsets are mapped back to a file line the way `spa_escaping_test` does
+/// it: count newlines in the prefix. Reporting the line within a normalised
+/// buffer would name a line that does not exist in the file.
+fn collapse_offsets(text: &str) -> Vec<usize> {
+    // Index from the normalised copy back to the original, so a match found
+    // without whitespace can still be reported where the reader will find it.
+    let mut flat = String::with_capacity(text.len());
+    let mut origin = Vec::with_capacity(text.len());
+    for (i, c) in text.char_indices() {
+        if c.is_whitespace() {
+            continue;
+        }
+        for lowered in c.to_lowercase() {
+            // One entry per BYTE of the lowered char, not per char:
+            // `match_indices` returns byte offsets into `flat`, so a
+            // char-indexed table drifts as soon as the file contains a
+            // multi-byte character — and these files do, in their prose.
+            let start = flat.len();
+            flat.push(lowered);
+            origin.resize(flat.len().max(start), i);
+        }
+    }
+    let mut out = Vec::new();
+    for (at, _) in flat.match_indices("bool_or(") {
+        let arg = &flat[at + "bool_or(".len()..];
+        let end = arg.find(')').unwrap_or(arg.len());
+        if arg[..end].rsplit('.').next() == Some("is_bot") {
+            out.push(origin[at]);
+        }
+    }
+    for (at, _) in flat.match_indices("havingnotbool_or") {
+        out.push(origin[at]);
+    }
+    out.sort_unstable();
+    out.dedup();
+    out
+}
+
 /// True if `line` collapses bot-ness to one value per canonical identity.
 ///
 /// Matched against a normalised copy — lowercased, whitespace removed —

docs/reports/deep_analysis_report.md

@@ -1835,4 +1835,57 @@ applies to its own gate.
     `actions/checkout` persisting credentials — and deserve their own pass
     rather than a blocking gate adopted in the same commit as the tool.
 
-The next sweep re-opens at **F307**.
+### F307 (Fixed — Unreleased) — the bot-filter matcher read one line at a time, and the codebase wraps long SQL
+
+*   **Location**: `codelore-lib/tests/bot_filter_hygiene_test.rs`
+*   **Severity**: LOW · **Category**: test reach / correctness
+*   **Defect**: F305 fixed the spelling axis — case, whitespace, table
+    qualifier — but normalised each line independently. A call wrapped across
+    lines is never assembled, so its argument is never seen. Confirmed by
+    planting `BOOL_OR(` with `a.is_bot` on the next line inside a SQL string
+    literal: the guard passed.
+*   **Not a hypothetical layout**: it is the house style in the very directory
+    the guard scans. `analyses/ownership.rs` writes `SUM(` on one line, its
+    argument on the next, and the closing paren on a third. A `BOOL_OR(` whose
+    argument grew long enough to wrap would be written the same way by the
+    same convention.
+*   **Fix**: normalise the whole file once and map matches back to a file line
+    by counting newlines in the prefix — the technique `spa_escaping_test`
+    already uses to report a file line from a statement offset.
+*   **A bug found while fixing it, worth recording because it is the same
+    class**: the first implementation indexed its offset table per *character*
+    while `match_indices` returns *byte* offsets, so the table drifted on any
+    file containing a multi-byte character — and these files contain them in
+    their prose. The guard detected the right thing and named the wrong line
+    (`soc.rs:142: _bot` instead of `soc.rs:141: BOOL_OR(`). A guard that names
+    the wrong line is a guard people stop trusting. Caught because the
+    regression test read the reported location rather than only the pass/fail.
+*   **Verified**: wrapped SQL now fails naming the construct's own line; all
+    four single-line spellings still caught; the tree is clean either way.
+
+### The Trusted Publishing deferral, corrected
+
+Cycle 13 tested the premise of §13's deferral and it does not hold as
+stated. The claim was that Trusted Publishing needs `id-token` on a job that
+runs `cargo publish`, which builds the crate and therefore executes
+`build.rs`. Only the *verification* step builds, and it is switchable:
+
+| command | `build.rs` executed |
+|---|---|
+| `cargo package --no-verify` | no |
+| `cargo package` | yes |
+
+Reproduced here independently on `cargo 1.97.1` (the cycle used 1.95.0), with
+the control that matters — a "no" with no corresponding "yes" would only mean
+the probe was broken.
+
+So the architecture is compatible: the build job keeps verification and runs
+repository code with no token, and a publish job holding `id-token` runs
+`cargo publish --no-verify`, executing none. **The deferral stands, but the
+reason was wrong.** The real blockers are that Trusted Publishing must be
+configured per-crate on crates.io — an action outside this repository — and
+that switching the workflow before that configuration exists breaks the next
+release. Sequencing, not incompatibility. Recorded so the next cycle does not
+re-derive a resolved argument.
+
+The next sweep re-opens at **F308**.