Merge pull request #313 from fiddlerwoaroof/chore/1xfb-5-1-repo-stats
chore(tooling): scripts/repo-stats — churn, coupling and doc-growth report (improvise-1xfb.5.1)
This commit is contained in:
@@ -1032,3 +1032,212 @@ changes. Two rivals were run:
|
||||
- reading every failure as stale fails the offline case (exit 1, wanted 2);
|
||||
- dropping `--locked` fails the stale case (exit 0, wanted 1). Without
|
||||
`--locked`, cargo quietly rewrites the lock and reports success.
|
||||
|
||||
## 7. The repo-stats pipeline (improvise-1xfb.5.1)
|
||||
|
||||
`scripts/repo-stats` reports churn, change coupling and doc growth over a window
|
||||
of history: by default the last 7 days of `gh/main`, falling back to `HEAD`,
|
||||
without merges. It reports and never gates. The user asked for code-maat to do
|
||||
the analyses it has, and for improvise to be the engine for the rest. So the
|
||||
pipeline is also an end-to-end run of improvise's headless product: `import`,
|
||||
`script` and `export`.
|
||||
|
||||
### The shape
|
||||
|
||||
1. `git log --numstat --no-renames --no-merges` in code-maat's `git2` format.
|
||||
2. code-maat runs `coupling` (with `--min-revs`/`--min-shared-revs`), `abs-churn`,
|
||||
`age` and `soc`. Each writes one CSV into the output directory, named after
|
||||
the analysis. code-maat is pinned in `flake.nix` as a `fetchurl` of the
|
||||
1.0.4 standalone jar, run by a JRE. It is in the dev shell and in the
|
||||
`.#repo-stats-tools` package, but not in the `ci` shell: the JRE is about
|
||||
0.6 GB on Linux, and every CI job enters that shell.
|
||||
3. The script adds `churn.csv`, one row per (commit, file) with the date, the ISO
|
||||
week, the path (`entity`, code-maat's own name for it), the area, the kind
|
||||
(prose, code or other), and the lines added and deleted. It also adds two
|
||||
flags to code-maat's `coupling.csv`: `crosses-crate` and `agent-rules`.
|
||||
4. `improvise import <dir>` (improvise-lpzu) merges the directory's five CSVs
|
||||
into **one** sparse model, `repo-stats.improv`. The merge's `File` category
|
||||
names each source, and the import makes one view per file, named after its
|
||||
stem and filtered to it. That is the user's ruling: "Records are already
|
||||
sparse: the model can just leave null/empty values for columns not in a
|
||||
given CSV file." A record keeps no coordinate in a category its file lacks.
|
||||
5. `improvise script` runs the checked-in `scripts/repo-stats.improvise`. It
|
||||
adds the prose-to-code formula, builds four views on top of the per-file
|
||||
ones and `export`s each to `report/*.csv`. The four names (`By area`,
|
||||
`By kind`, `Area by week`, `By day`) must never be a file's stem:
|
||||
`create-view` of an existing name replaces that view silently (gap 17). The
|
||||
suite's view count, 10, fails if one does. None of the report reads the
|
||||
merged `Default` view, which can be empty for a typical code-maat pair
|
||||
(improvise-8jk6.15).
|
||||
6. The text report is those exports laid out as tables. The code-maat rankings,
|
||||
doc sizes and the coupling flags are added to it. `--json` gives the same
|
||||
data as JSON.
|
||||
|
||||
The `.improv` file is the artifact. It opens in the TUI, and CI uploads it with
|
||||
the CSVs.
|
||||
|
||||
**A window in which no file changed builds no model.** Its commits are all
|
||||
empty (`--allow-empty`), so there is no churn to import and nothing for
|
||||
code-maat to analyse. The script writes `churn.csv` with its header alone, skips
|
||||
code-maat and improvise, and the report says so. It exits 0. Running improvise
|
||||
over an empty CSV was the other option. It was not taken, because it would test
|
||||
only how improvise treats a CSV with no records, which is not what this pipeline
|
||||
exists to exercise.
|
||||
|
||||
**`--out` must hold no CSV the script did not write.** `improvise import .`
|
||||
merges every CSV in the directory, so a stale or foreign one would silently
|
||||
join the model. The script refuses one with exit 2 and names it. It refuses
|
||||
rather than deleting, because deleting files in a directory the caller named is
|
||||
the wrong default. Its own five CSVs are overwritten on every run, so a rerun
|
||||
into the same `--out` works.
|
||||
|
||||
**No collisions, by construction.** A `churn.csv` record is keyed by commit and
|
||||
file, which `--no-renames` makes unique. code-maat's records are keyed by
|
||||
entity, by entity pair or by date. `File` separates the sources. So the import
|
||||
keeps its default, `--on-duplicate refuse`. A collision would be a bug in this
|
||||
script, and the import refuses it.
|
||||
|
||||
**What improvise computes and what it does not.** Everything that is a sum or a
|
||||
ratio of sums is a view:
|
||||
- churn by area, with `DeletionRatio = deleted / added` as a `_Measure` formula;
|
||||
- prose, code and other, with `ProseToCode = prose / code` as a formula on the
|
||||
`kind` category;
|
||||
- area by ISO week;
|
||||
- lines added per day from `churn.csv` next to code-maat's `abs-churn`. The two
|
||||
columns must agree, and the suite checks that they do.
|
||||
|
||||
Two kinds of output stay in Python. The ranked code-maat tables (coupling, soc
|
||||
and age) are read from code-maat's own CSVs, because improvise cannot sort rows
|
||||
(gap 11 below). Doc sizes are a property of files at one ref, not of the window.
|
||||
|
||||
### Definitions
|
||||
|
||||
- **Areas**: first match wins, in the order of `AREAS` in the script.
|
||||
`generated` comes first, so `harness/opfs-contract/Cargo.lock` counts as
|
||||
generated and not as harness. `.beads/` and `site/` are generated too.
|
||||
`docs/*.md`, `README.md` and `RELEASING.md` are "other docs", and `.codex/`
|
||||
is agent docs and skills. A Rust test is a `.rs` file under a `tests/` or
|
||||
`benches/` directory, or one named `tests.rs` or `*_test(s).rs`. A
|
||||
`#[cfg(test)] mod tests` inside a source file counts as Rust src, because no
|
||||
path can tell the two apart.
|
||||
- **Prose** is `context/code`, `context/test-evidence`, other `context/`, other
|
||||
docs, `repo-map`, and the agent docs and skills. **Code** is Rust src, Rust tests,
|
||||
scripts/CI, and web and harness. `kb.pl`, generated files and other files are
|
||||
neither.
|
||||
- **The deletion ratio** is deleted lines over added lines. It is 0 when nothing
|
||||
was deleted, and blank when nothing was added.
|
||||
- **A crate crossing**: both paths are in a crate (`crates/<name>/`, the root
|
||||
crate's `src/`, `tests/`, `benches/`, `examples/` or `Cargo.toml`, or
|
||||
`harness/opfs-contract/`), and the two crates differ. A pair with one side
|
||||
outside every crate is not flagged.
|
||||
- **Agent rules**: both paths are in the agent docs and skills area. Such a pair
|
||||
means one rule stated in several places.
|
||||
- **Gotchas** are the numbered items between `## Gotchas` and the next `---` or
|
||||
`##`. The longest gotcha is measured in characters. Most gotchas are one long
|
||||
line, so a line count would say little.
|
||||
- **Test-evidence entries** are the `###` and `####` headings.
|
||||
|
||||
### What fails, and what never does
|
||||
|
||||
- **The numbers never fail anything.** No budget is checked. Budgets are a
|
||||
separate decision under the compaction epic, improvise-1xfb.5.
|
||||
- **improvise refusing the pipeline's input exits 3.** That covers a non-zero
|
||||
exit from `import` or `script`, and any `warning:` on their stderr. The
|
||||
warning rule is there because a skipped record is data loss that exits 0
|
||||
(gaps 3 and 4). It is a product regression, not a statistic.
|
||||
- **In CI**, the suite runs on every PR in `nix-build` and gates like any other
|
||||
suite. The report runs only on a push to `main`. Its step has
|
||||
`continue-on-error`, so an improvise failure there marks the step failed and
|
||||
the job and the deploy go on. Whether that failure should block the deploy
|
||||
is a question for the user on the PR. The report appends to
|
||||
`$GITHUB_STEP_SUMMARY`, and the directory is uploaded as the `repo-stats`
|
||||
artifact.
|
||||
- The report runs in `nix-build` because that job already builds
|
||||
`.#improvise`. Its steps run under `nix shell .#repo-stats-tools`, which joins
|
||||
that same build with code-maat, so nothing is built twice. It is one package
|
||||
rather than two installables because `check-ci-graph.py` reads a counted
|
||||
step as `nix shell <one> --command`. The job's checkout fetches full history
|
||||
(`fetch-depth: 0`) for the report.
|
||||
|
||||
### Gaps in the headless product, and the workarounds used
|
||||
|
||||
Each of these was found by running the pipeline, and each is a finding against
|
||||
improvise's command and model layer. Where a workaround exists, the script
|
||||
names it.
|
||||
|
||||
1. **A forced `--category` value is still parsed as a number.** The commit
|
||||
`00565796` imported as item `565796`, and `220944e2` imported as
|
||||
`22094400.0`. Workaround: `churn.csv` writes commits as `g<sha>`.
|
||||
improvise-8jk6.4.
|
||||
2. **`import --formula` drops a formula that fails to parse, and exits 0.**
|
||||
`--formula 'X = ('` saves a model with no formula and says nothing.
|
||||
`add-formula` in a script refuses it and exits 1, so the pipeline's own
|
||||
category formula is added there. improvise-8jk6.5.
|
||||
3. **A record that lacks any category's value is skipped.** A merge of CSVs with
|
||||
different headers, which is the user's sparse model, imports **no records**.
|
||||
Closed by improvise-lpzu, whose sparse import this pipeline now uses. Until
|
||||
it landed, every CSV was padded with `-` for the categories it lacked. A
|
||||
related hole is still open: a record with no measure value stores nothing
|
||||
(improvise-8jk6.14).
|
||||
4. **An import that skipped records exits 0**, even when it skipped all of
|
||||
them. It saves an empty model and prints only a warning. improvise-8jk6.6.
|
||||
5. **`set-axis` and `set-page` accept names that do not exist, and exit 0.** An
|
||||
unknown category, an unknown axis name (`rwo`) and an unknown item are all
|
||||
silent no-ops. improvise-31oe.2.
|
||||
6. **The command language does not read `|pipe quoting|`.**
|
||||
`create-view |By area|` made a view named `|By`, and the second word was
|
||||
ignored. Commands that take a fixed number of arguments ignore extra ones
|
||||
without saying so. Workaround: `"double quotes"`. improvise-31oe.3.
|
||||
7. **`create-view` puts every category it is not told about on Page**, with its
|
||||
first item selected. That silently filters the view. Every view has to set
|
||||
every category's axis, one `set-axis` line each. improvise-31oe.4.
|
||||
8. **A view cannot hide a category's column.** A per-file records view shows
|
||||
every category and measure in the model. improvise-31oe.5.
|
||||
9. **`export` of a records view writes the editor's `+cat` and `+` columns.**
|
||||
improvise-8jk6.7.
|
||||
10. **`export` writes display text.** It uses the view's number format, so the
|
||||
import's default `,.2` gives `"17,512.00"`. Workaround: `set-format .0` on
|
||||
every exported view, and `FORMAT .3` on the ratio formulas.
|
||||
improvise-8jk6.8.
|
||||
11. **Nothing sorts.** Items stay in first-seen order, so weeks come out
|
||||
newest first. A top-N table cannot be produced, so code-maat's rankings are
|
||||
read from its CSVs. improvise-31oe.6.
|
||||
12. **`--extract` has no ISO week.** It has year, month and quarter. Workaround:
|
||||
`churn.csv` carries a `week` column. improvise-8jk6.9.
|
||||
13. **`import --formula` targets only `_Measure`.** A formula on another
|
||||
category needs `add-formula <category> …` in a script. improvise-8jk6.10.
|
||||
14. **The saved model opens on the import's `Default` view**, which puts `File`
|
||||
on rows and `area` on columns, and not on the view the script left active.
|
||||
That is improvise-azhb.
|
||||
15. **No folder import.** Closed by improvise-lpzu (PR #314), which this
|
||||
pipeline now uses. Until it landed, the script named the five files and
|
||||
generated its own per-file views.
|
||||
16. **There is no headless output but a file.** `export` writes a CSV, and the
|
||||
tables are laid out in Python. improvise-31oe.7.
|
||||
17. **`create-view` of an existing name replaces that view, and exits 0.**
|
||||
`Workbook::create_view` inserts into an `IndexMap`, so it overwrites without
|
||||
a word. Folder import makes a view per file stem, so a report view named
|
||||
like a stem would replace that file's view.
|
||||
improvise-31oe.8.
|
||||
|
||||
### The suite
|
||||
|
||||
`scripts/repo-stats-test.sh` builds a fixture repository whose every number is
|
||||
known, runs the whole pipeline, code-maat and improvise included, and asserts
|
||||
improvise's exported tables. It runs in `nix-build`, which puts improvise and
|
||||
code-maat on `PATH`. Locally it uses improvise from `PATH`, or else
|
||||
`target/debug/improvise`, and it fails with a message when there is neither.
|
||||
The fallback does not check how fresh that binary is, so locally the suite can
|
||||
test a `target/debug/improvise` older than the tree. Rebuild after changing
|
||||
improvise.
|
||||
|
||||
It does not build improvise itself. `checkers.sh` scrubs the environment to an
|
||||
allowlist, and on macOS that removes what the linker needs: a `cargo build`
|
||||
inside the suite failed with `library not found for -liconv`. That is a bug in
|
||||
`checkers.sh`: its allowlist drops the Nix linker variables, and the
|
||||
orchestrator has filed it separately. Build first with
|
||||
`cargo build --bin improvise`; `cargo test --workspace` also leaves the binary
|
||||
there. The suite is the slowest in `checkers.sh`, at about 25 seconds, most of
|
||||
it JVM start-up for code-maat's four analyses per run.
|
||||
|
||||
The evidence and the rivals are in `context/test-evidence/tooling.md`.
|
||||
|
||||
@@ -1811,3 +1811,107 @@ that failed would leave the owner-authored base at HEAD and fail the case too.
|
||||
`GIT_CONFIG_GLOBAL=/dev/null` and `GIT_CONFIG_NOSYSTEM=1`, and every `GIT_*`
|
||||
identity variable is unset first. So a caller's own identity cannot make the
|
||||
author or committer assertions agree by accident.
|
||||
|
||||
### The repo-stats pipeline computes what it claims to — `scripts/repo-stats-test.sh` (improvise-1xfb.5.1)
|
||||
|
||||
**What it pins.** The fixture repository has one commit before the window, a
|
||||
side branch merged with `--no-ff`, a shrink in the next ISO week, and one file
|
||||
in every area. So each number the report prints is known by construction. The
|
||||
suite runs the real pipeline, code-maat and improvise included, and asserts
|
||||
improvise's exported tables: each area's added, deleted and deletion ratio;
|
||||
prose, code and the `ProseToCode` row; the week split; and the per-day totals.
|
||||
It also checks the doc sizes and the coupling flags, and that an improvise that
|
||||
refuses or warns makes the pipeline exit 3.
|
||||
|
||||
**Two fixture choices are load-bearing.**
|
||||
- The repo map's `4.` item sits after the `---` that closes the Gotchas
|
||||
section, with no `##` heading between them. In an earlier fixture a `##`
|
||||
heading followed the `---`, so the `---` rule could be deleted and the suite
|
||||
stayed green.
|
||||
- `churn.csv` must equal code-maat's `abs-churn` on every day. That compares
|
||||
two independent derivations of one number, one of them improvise's
|
||||
aggregation. The fixture has a binary-free, rename-free history, so the two
|
||||
agree exactly.
|
||||
|
||||
**Rivals**, each injected into the script or into `repo-stats.improvise` and run
|
||||
through `checkers.sh repo-stats`:
|
||||
- Rust src matched before Rust tests: the rust src and rust tests rows fail.
|
||||
- `DeletionRatio = deleted / (added + deleted)`: six ratio rows fail.
|
||||
- `ProseToCode = code / prose`: the `ProseToCode` row fails.
|
||||
- `--no-merges` dropped: the commit count fails (6, wanted 5).
|
||||
- The `---` end of the Gotchas section ignored: the gotcha count fails (4, wanted 3).
|
||||
- A crate crossing as "the crates differ", so a crate against no crate counts:
|
||||
the crate-crossing case fails.
|
||||
- `kb.pl` counted as prose: the prose row and the ratio fail.
|
||||
- The category padding removed: the import skipped every record, and both
|
||||
pipeline runs failed with exit 3. That rival is retired: folder import
|
||||
(improvise-lpzu) keeps sparse records, and the padding is gone. The suite
|
||||
passes 41 of 41 without it.
|
||||
- Months instead of ISO weeks: both week cases fail.
|
||||
- A `warning:` from improvise ignored: the warning-stub case fails.
|
||||
|
||||
### A window of empty commits, and four faults the first fixture missed — `scripts/repo-stats-test.sh` (improvise-1xfb.5.1)
|
||||
|
||||
**What broke.** ChatGPT Codex found it on PR #313, and the adversarial review
|
||||
reproduced it. When every non-merge commit in the window is `--allow-empty`,
|
||||
the log has commit headers and no files. `build_dir` took `churn.csv`'s header
|
||||
from `rows[0]`, raised `IndexError` and exited 1. That exit is outside the
|
||||
pipeline's failure split: it is neither a number nor improvise's exit 3.
|
||||
|
||||
**Fix.** `churn.csv` has a fixed header. When no file changed, the script
|
||||
skips code-maat and improvise, writes the header alone, and exits 0.
|
||||
|
||||
**Red first.** The new case is a second fixture repository whose only
|
||||
in-window commit is empty. Restoring the `rows[0]` header fails its run case
|
||||
with the traceback.
|
||||
|
||||
**The review's four surviving faults, each now killed:**
|
||||
- **`--no-renames` dropped.** The fixture now renames `scripts/foo.sh` to
|
||||
`scripts/bar.sh`. Without the flag git reports one `{foo.sh => bar.sh}` entity
|
||||
with no lines changed, not a delete and an add. Four cases fail: the scripts/CI
|
||||
row, the code row, the ratio row and the coupling count.
|
||||
- **`crate_of` forgetting the root crate.** `src/main.rs` now co-changes with
|
||||
both crates. The crossing list loses three pairs.
|
||||
- **A failing `script` call ignored.** A third stub exits 0 for `import` and
|
||||
refuses `script`. The pipeline then reads exports that were never written and
|
||||
exits 1, not 3.
|
||||
- **`.beads/` and `site/` left out of generated.** Both are in the fixture now.
|
||||
The generated row and the other row both fail.
|
||||
|
||||
**Also counted now:** `docs/guide.md` as other docs and `.codex/config.toml` as
|
||||
agent docs. The coupling count is 16, not 15: `web/app.js` and
|
||||
`scripts/foo.sh` change together in two commits, the 06-04 one and the rename.
|
||||
|
||||
### The pipeline on folder import — `scripts/repo-stats-test.sh` (improvise-1xfb.5.1)
|
||||
|
||||
Once #314 merged, `improvise import <dir>` replaced the file list, the `-`
|
||||
padding and the generated per-file views. The suite needed no change of
|
||||
expectations and still passed 41 of 41. That shows only that the numbers
|
||||
agree. It does **not** show that the model is right: the suite could not tell
|
||||
sparse from padded. The delta review re-added `pad_categories` and the suite
|
||||
still passed 41 of 41, while the model then had a `|-|` item in 13 places and
|
||||
the `churn` view seated `agent-rules` on rows and paged everything else. The
|
||||
view case counted 10 views either way.
|
||||
|
||||
**What separates them now:**
|
||||
- **"the sparse model has no padding item"**: no line of the model names `|-|`.
|
||||
- **"the churn view seats …"**: four cases, one per seat. `date` is on rows, and
|
||||
`agent-rules`, `coupled` and `crosses-crate`, which are coupling-only, are on
|
||||
`none`. The churn view puts `entity` on columns, not `area` as the review
|
||||
guessed. What discriminates is `date` on rows and the coupling-only
|
||||
categories on `none`, which a padded CSV cannot give, because it claims all
|
||||
nine categories.
|
||||
|
||||
**Rivals:**
|
||||
- **Padding re-added** (the review's own injection): 5 cases fail. They are
|
||||
the `|-|` case, with 161 lines naming it in the fixture model, and all four
|
||||
seat cases.
|
||||
- **A report view named like a file stem**: `By day` renamed to `churn` leaves
|
||||
9 views, and the view-count case fails.
|
||||
- **The foreign-CSV guard removed**: "a foreign CSV in --out is refused with
|
||||
exit 2" fails with exit 0. The guard is the answer to the review's
|
||||
non-blocking finding that `import .` picks up any CSV in `--out`. It refuses
|
||||
instead of clearing. Deleting files in a directory the caller named is the
|
||||
wrong default, and the script's own five CSVs are overwritten on every run,
|
||||
so a rerun into the same `--out` is still accepted. The same fixture run
|
||||
writes JSON and then text into one directory, which shows that.
|
||||
|
||||
Reference in New Issue
Block a user