Skip to content

fix(ci): make check-cycles honest — real graph walk + cycles ratchet (#15159 G-01/G-02) #15281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
diegosouzapw merged 1 commit into from
Oct 2, 2026
Merged

fix(ci): make check-cycles honest — real graph walk + cycles ratchet (#15159 G-01/G-02) #15281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ jobs:
path: .artifacts/eslint-results.json
if-no-files-found: warn
retention-days: 7
- run: npm run check:cycles
- run: npm run check:cycles:ratchet
- run: npm run check:route-validation:t06
- run: npm run check:any-budget:t11
- run: npm run check:provider-consistency
Expand Down
3 changes: 2 additions & 1 deletion AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ npm run typecheck:core # TypeScript check (should be clean)
npm run typecheck:noimplicit:core # Strict check (no implicit any)
npm run test:coverage # Unit tests + coverage gate (60/60/60/60 — statements/lines/functions/branches)
npm run check # lint + test combined
npm run check:cycles # Detect circular dependencies
npm run check:cycles # Detect circular dependencies (advisory — lists the SCCs)
npm run check:cycles:ratchet # Same scan, blocking above the quality-baseline ceiling
npm run check:docs-all # Run after changing documentation (includes fabricated-docs validation)
```

Expand Down
7 changes: 7 additions & 0 deletions config/quality/quality-baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,13 @@
"direction": "down",
"dedicatedGate": true
},
"cycles": {
"value": 14,
"direction": "down",
"dedicatedGate": true,
"_note": "Import-cycle count over src/ + open-sse/ from scripts/check/check-cycles.mjs (#15159 G-02). Before this entry the metric did not exist and the gate was a false green: it scanned 5 subdirectories (450 files), matched only static `import|export … from`, and dropped every `@/` and `@omniroute/open-sse/` specifier — so it could not see the dynamic-import + alias cycles that dominated the repo. Fixed in the same change; the 14 below are the cycles the honest scan finds at release/v3.8.52 @ dbe703a000, not new debt.",
"_seed_2026_10_01_g01_g02_false_green": "0 (unmeasured, falsely reported) -> 14. The previous 0 was not a measurement, it was a blind spot. Fixing the three blinds (roots -> src+open-sse, AST-based extraction so `import(\"…\")` counts while type-position `typeof import(\"…\")` does not, tsconfig paths resolution for `@/*` and `@omniroute/open-sse/*`) took the scanned surface from 450 to 5023 files and surfaced 14 strongly connected components. Largest is a 43-file SCC over src/lib/db (settings.ts -> readCache.ts -> settings.ts, closed by the dynamic `await import(\"@/lib/db/settings\")` at src/lib/db/readCache.ts:102 — exactly the shape the old regex + alias filter could not see). Ceiling seeded at the measured 14 so the metric can only fall; burn-down is A-01 (chatCore.ts split) and the open-sse/services+imageGeneration extractions. Measured twice, identical both times. Do NOT raise this ceiling to make a gate green — that is the exact inverse of this ratchet."
},
"bundleSize": {
"value": 10384,
"direction": "down",
Expand Down
4 changes: 2 additions & 2 deletions docs/architecture/QUALITY_GATES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ Runs on every PR to `main`. Blocks merge on failure.
| Script (`npm run ...`) | Validates | Blocking |
| --------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------- |
| `check:node-runtime` | Node.js version is within the supported range | Yes |
| `check:cycles` | Circular imports — all `src/` + `open-sse/` modules | Yes |
| `check:cycles` | Circular imports across all of `src/` + `open-sse/` (AST-based, tsconfig `paths` resolved). Bare = advisory, lists the cycles. `check:cycles:ratchet` (what CI runs) blocks when the count exceeds the `metrics.cycles` ceiling in `quality-baseline.json` — currently 14, `direction: down`, so it can only fall (#15159 G-01/G-02) | Yes (ratchet) |
| `check:route-validation:t06` | Zod schemas present on all routes (Tier 6 policy) | Yes |
| `check:any-budget:t11` | `@ts-expect-error // any` count does not exceed budget (Tier 11 catraca) | Yes |
| `check:provider-consistency` | Every provider in `providers.ts` has a matching entry in `providerRegistry.ts` (and vice-versa, within the allowlist) | Yes |
Expand Down Expand Up @@ -517,7 +517,7 @@ several "obvious" merges turned out to hide debt and are **not** clean drop-ins.

- **`check:docs-sync` runs twice** — standalone in the `lint` job and again inside `check:docs-all` (`docs-sync-strict`) and the husky pre-commit hook. ✅ **DONE** — standalone `lint` invocation removed.
- **CVE scanning** — ❌ **NOT a clean merge.** `audit:deps` hard-fails on any high/critical CVE; `check:vuln-ratchet` (osv) only fails on a _regression_ vs baseline (currently 1 MODERATE). Different semantics — dropping `audit:deps` would lose the absolute high/critical gate. Keep both.
- **Cycle detection** — ❌ **NOT a clean merge.** `check:circular-deps` (dpdm) reports **91 cycles** (that is why it is advisory); it cannot be promoted to blocking without first resolving them, and it has a broader scope than the green, curated `check:cycles`. Keep `check:cycles` blocking; resolving the 91 dpdm cycles is its own backlog.
- **Cycle detection** — ✅ **DONE** (#15159 G-01/G-02). The old text here called `check:cycles` "the green, curated" gate and justified keeping it blocking because `check:circular-deps` (dpdm) reported 91 cycles. That green was a **false green**: `check:cycles` scanned 5 subdirectories (450 files), matched only static `import|export … from`, and dropped every `@/` and `@omniroute/open-sse/` specifier, so it could not see the dynamic-import + alias cycles that dominated the repo. Fixed: the gate now walks `src` + `open-sse` (5023 files), collects specifiers from the TypeScript AST (so `import("…")` counts and type-position `typeof import("…")` does not), and resolves tsconfig `paths`. It finds **14** cycles, not 0. Because 14 pre-existing cycles cannot be fixed in a gate PR, `check:cycles` is now a **ratchet** (`--ratchet`, ceiling `metrics.cycles.value = 14` in `quality-baseline.json`, `direction: down`) — it blocks any _regression_ and the count can only fall. CI runs `npm run check:cycles:ratchet`. Burn-down rides with **A-01**. `check:circular-deps` (dpdm) stays advisory as the broader second opinion.
- **Complexity** — ✅ **DONE** (`check:complexity-ratchets` / `eslint.complexity-ratchets.config.mjs`): one ESLint walk, counts by ruleId so cyclomatic+max-lines and cognitive baselines stay independent; individual `check:complexity` / `check:cognitive-complexity` remain for local `--update`.
- **`/api` anti-hallucination** — ✅ **DONE** (`check:api-docs-refs` + `scripts/check/lib/apiRoutes.mjs`): one FS inventory of `src/app/api`, openapi-routes + docs-symbols still report independently; individuals remain for local runs.
- **`check:node-runtime` runs in 11 jobs** — ⚠️ **low ROI.** Each is a separate runner and the check is <1s; total savings ~10s, against losing a cheap per-job guard. Not worth the churn.
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@
"test:security": "cross-env DISABLE_SQLITE_AUTO_BACKUP=true node --import tsx/esm --import ./open-sse/utils/setupPolyfill.ts --import ./tests/_setup/isolateDataDir.ts --test tests/unit/security-fase01.test.ts",
"test:property": "cross-env DISABLE_SQLITE_AUTO_BACKUP=true node --import tsx/esm --test --test-force-exit tests/unit/correctness/*.property.test.ts",
"check:cycles": "node scripts/check/check-cycles.mjs",
"check:cycles:ratchet": "node scripts/check/check-cycles.mjs --ratchet",
"check:route-validation:t06": "node scripts/check/check-route-validation.mjs",
"check:any-budget:t11": "node scripts/check/check-t11-any-budget.mjs",
"check:docs-sync": "node scripts/check/check-docs-sync.mjs",
Expand Down
Loading
Loading