ADORSYS-GIS / ADORSYS-GIS/lightbridge-governance
[Epic]: Code-quality ratchet — 200 LoC per file, SOLID, DRY
- Lingua principale
- Rust
- Stelle
- 1
- Fork
- 2
- Merge medio
- 13h 13m
- PR unite (30g)
- 110
Descrizione
> ## 📌 Sprint tracker — the `Estimate` on this epic is deliberately `0`
>
> This epic sits in **Sprint 26-08** as a **tracker**, not as work.
>
> Its board `Estimate` is `0` **on purpose**. Every point of this epic's work is
> estimated on its child tickets. If the epic also carried its rollup, a sprint holding
> both the epic and its children would report roughly double the work actually committed —
> which is how a capacity number stops meaning anything.
>
> | | |
> |---|---|
> | **True rollup (sum of children)** | **34 points** |
> | In Sprint 26-08 | 2 points — #172 (2) — the CI gate only |
> | Not in this sprint | #173-#180 (32) — the burn-down, which runs alongside feature work rather than as a sprint push |
>
> **Reading the board:** sum the **children**, never the epic rows. An epic row showing
> `0` does not mean the work is free; it means the work is counted one level down.
>
> Progress is tracked by the sub-issue list below, not by this row's estimate.
---
### Executive Summary
Ratchet this repository to a 200-LoC-per-file ceiling with SOLID and DRY as review criteria,
mirroring `lightbridge-authz`#506 so both repos ratchet the same way.
**Measured baseline (2026-08-27): 50 of 93 Rust files exceed 200 LoC, holding 26,467 of
30,771 total lines — 86% of the codebase.**
### Strategic Intent
Large files are where review quality goes to die: a 1,500-line module is not reviewed, it is
skimmed. This platform is security-adjacent — `/internal/v1/resolve` decides whether telemetry
is accepted — and the failure modes that matter most are the ones a skimmed diff hides.
### Problem Statement
Worst offenders today:
| File | LoC |
|---|---|
| `crates/governance-core/tests/migrate_and_create.rs` | 1613 |
| `app/redact-extproc/src/service.rs` | 1588 |
| `app/governance-auth/src/otel.rs` | 1522 |
| `app/governance-auth/src/config.rs` | 1351 |
| `crates/governance-core/src/ingest.rs` | 1278 |
| `app/governance-ctl/src/sync.rs` | 1178 |
| `app/lightbridge-governance/src/metrics.rs` | 1119 |
⚠️ **The raw number overstates the production-code debt.** Much of it is inline
`#[cfg(test)]` — `config.rs` is 1351 lines of which roughly 840 are its precedence test
module. Extracting tests is cheap and low-risk; splitting production modules is not. The
baseline story must report those two numbers separately before anyone commits to a burn-down.
### Desired Outcome
New and changed files stay under 200 LoC because CI says so, and the existing debt burns down
against a published baseline.
### Scope (In / Out)
**In** — the CI gate; a published baseline split production-vs-test; a SOLID/DRY reviewer
checklist in the PR template; splitting the worst modules; extracting inline test modules.
**Out** — behaviour changes of any kind. Every split is a pure refactor, and the tests that
pass before must pass after, unchanged.
### Source of truth (links)
- `lightbridge-authz`#506 (the sibling ratchet), #516 (CI gate), #517 (baseline), #518
(reviewer checklist) — copy their shape rather than inventing a second one
- `AGENTS.md` — "Spend review attention here instead": the list of things no lint can judge
### Stakeholders
Every contributor; reviewers most of all.
### Key Assumptions
1. A CI gate on new/changed files is achievable without blocking the existing 50.
2. Splitting a module is behaviour-preserving and provable by the existing suite.
### Constraints
- **Stop the bleeding first.** The CI gate lands before any burn-down work — it is roughly a
day and prevents the problem growing while the rest is negotiated.
- Suppressions use `#[expect(..., reason = "...")]`, never `#[allow]`.
- No test is weakened to make a split pass. A shrinking file that also shrinks its assertions
is a regression wearing a refactor's clothes.
### Risks
- A mechanical LoC gate rewards splitting a file into arbitrary halves. The reviewer checklist
is what makes the split meaningful; without it the metric is gameable and will be gamed.
- 86% of the codebase is in scope. This is a ratchet, not a sprint — it runs alongside feature
work, permanently.
### Success metrics
- CI blocks a new file over 200 LoC (proven by trying one).
- The baseline is published and trending down.
- No file added after the gate lands exceeds the ceiling.
### Child User Stories
See linked sub-issues.
### Human accountable owner
@stephane-segning
### AI Usage Declaration
Drafted by Claude Code (Opus 5). The baseline was measured with `wc -l` over
`crates/` and `app/` at commit `1c3df7d`, not estimated. The test-vs-production caveat comes
from reading `config.rs`, not from assumption.
### Human verification completed
Not yet.
Guida per i contributori
Apri la guida per i contributori
Valutazione
Questa issue non è ancora stata valutata.