ADORSYS-GIS / ADORSYS-GIS/lightbridge-governance

[Epic]: Code-quality ratchet — 200 LoC per file, SOLID, DRY

Aperta
#162 2 commenti 0 reazioni 1 assegnatario Rivendicata da @Koufan-De-King Vedi su GitHub
epic
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.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.