perf(core-api): load the ruleset corpus once, and make the corpus load clean (GT-648, GT-649) - #331
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📊 Bilingual Coverage ImpactPR Changes
Repository Coverage
To create skeletons: node .harness/scripts/generate-es-skeleton.mjs <file.md>Generated by GitHub Actions |
…-648) Advances GT-648 `RuleEvaluationEngine.discoverAndEvaluate` calls `loadAllRulesets(corePath)` on every evaluation, and core-api's `IRulesetRepository` was a bare `DiskRulesetRepository` with no cache. So each `POST /api/v1/evaluate` walked the corpus tree, read 176 `*.rules.json` files, and ran every one through Ajv to produce the same 393 rules as the request before it. Both halves are synchronous CPU work inside `await`ed calls, so nothing else on the event loop advanced while they ran — which makes it a whole-process latency defect, not a slow endpoint. CI run 30631939687 (Reliability workflow) shows both halves: the corpus's load-time WARN block repeats once per k6 iteration, and a `GET /health` that landed during that work took 498.5 ms end-to-end — `http_req_waiting` 498.4 ms against `http_req_connecting` 0.17 ms — while core-api logged its own handler at `durationMs=0`. The request was never slow; it was queued behind a blocked loop. Reproduced against `origin/develop` at 3051e49 before changing anything: eleven evaluations produced exactly eleven corpus loads, and one `loadAllRulesets` call cost 30.4–54.2 ms with a 10 ms timer queued across it firing up to 14 ms late. That laptop figure is roughly a tenth of the runner's and the gap is the point — a warm page cache on an SSD is the best case for exactly this work. The cache is a decorator (`CachingRulesetRepository`), not a field on the disk repository, because "load once" is a property of THIS deployment: the CLI is a one-shot process that must keep re-reading disk between invocations. Three properties are load-bearing and each is pinned by a test — the in-flight promise is memoized so concurrent cold requests do not each start their own walk; failures are never cached, so GT-474 keeps its meaning and a repaired corpus loads on the next attempt instead of latching the process into permanent failure; and each caller gets its own array, because the coverage helpers are free to sort what they are handed. A cache alone would still leave request #1 — the one a readiness probe lets through — paying the load, so `RulesetCorpusWarmupService` reads the corpus at `OnApplicationBootstrap` and logs the count as the evidence that it happened once: `Ruleset corpus loaded at startup: 393 rules … in 181 ms`. Also per request, and compounding it: `SdlcDataLoaderService.loadAllGates` re-read the phase index once per phase (`loadGatesForPhase` → `loadPhase` → `loadAllPhases`), turning one scan into N+1. Memoized per instance. Verdicts are unchanged, verified rather than argued. The full `EvaluationResult` for the k6 smoke payload was captured from a build of `origin/develop` and from this change, and the two are byte-for-byte identical at 442,204 bytes. The normalized corpus was diffed against the pre-change loader separately: 393 rules, identical JSON. Measured after: evaluate median 52.7 → 38.2 ms, and a `/health` issued 5 ms into an in-flight evaluate 11.6 → 3.5 ms. Still to be re-measured on a runner, where the 498 ms was observed — local numbers understate this by ~10x by construction. The lockfile line is a real pre-existing drift, not a regeneration: `infra-providers/package.json` has declared `@beyondnet/evolith-contracts` while the lockfile did not record it, which is the `Cannot find module '@beyondnet/evolith-contracts/ingest'` that keeps surfacing in CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…649)
Advances GT-649
Three faults surfaced together in the core-api.log of CI run 30631939687. They
looked like log noise. Two of them are, and the third is a governance stage that
has evaluated nothing for four weeks.
THE PHASE GATES HAVE BEEN UNREACHABLE SINCE 2026-07-04. `f0d01911`
("remove leftover old directories under reference/") deleted the whole
`reference/governance/` tree, including the five `phase-f*.json` files; GT-461
later re-created `gates/` at that same path and did not re-create `phases/`.
Because `loadGatesForPhase` resolves gates THROUGH the phase index,
`loadAllPhases()` returning `[]` means all five authored gate files are
unreachable — so `SatelliteEvaluationPipeline`'s phase-gate stage produced zero
gate results, reported no error, and the top-level verdict was computed without
it. The only outward sign was one WARN line per evaluation.
Restoring the data is therefore a behavioural change and is meant to be read as
one: `POST /api/v1/evaluate` now returns `gate-f1` alongside `general-rulesets`
where develop returns only the latter, and a satellite that was passing because
its F1 gate never ran can now legitimately FAIL. For the k6 smoke payload the
top-level verdict is unchanged (FAIL either way, already failing on general
rulesets) — but that payload does not exercise the flip and should not be read
as evidence that none does. The board row carries this as an open criterion.
THREE FILES WERE REPORTED BROKEN AND WERE NOT. `topology-recommendation`,
`helm-enforcement` and `opa-sidecar-bundle` are different document kinds that
share the `*.rules.json` suffix — an ADR-0104 recommendation catalogue with its
own consumer, and two single-rule declarations enforced by guard 29 and their
paired Rego. The loader had no vocabulary for "not a corpus ruleset", so it
checked each against a schema it was never meant to satisfy and reported the
result as breakage, on every load. It now classifies by the `$schema` a document
declares and validates each against THAT contract — strictly more checking than
before, since the old path checked the wrong schema and discarded the answer. A
document that violates its own declared schema still warns, and a
standard-shaped ruleset that fails the standard schema still warns; both
directions are pinned, so silencing did not become blindness.
All three also failed the schema they themselves declare, unnoticed, because
nothing had ever checked them: `severity: "error"` is not in that enum, the
field is `title` and the files wrote `name`, the `validation` block they have
carried from the start was never modelled, and the `$schema` and target paths
were left pointing at `reference/infrastructure/…`, a tree the same 2026-07-04
commit removed. Fixed in both directions — the schema now models `validation`,
and the files name paths that exist.
EVERY SINGLE-ARGUMENT LOG LINE WAS PRINTED TWICE. Nest's `Logger` reads
`...optionalParams` positionally, so `warn(message, undefined)` is not
`warn(message)` — the explicit `undefined` becomes a second thing to print.
44 `WARN undefined` lines across an 11-request run, now 0.
Verified against the real corpus rather than a fixture, because a fixture cannot
tell us the tree as authored is clean: it loads with zero warnings and zero
errors, and the 393 normalized rules are byte-identical to before.
`iso-5055-mapping.json` is regenerated: it reads the two INFRA declarations
directly and picks up their corrected severities.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
beyondnetPeru
force-pushed
the
fix/gt-648-649-corpus-load-once
branch
from
July 31, 2026 23:04
1d94795 to
5c9654d
Compare
beyondnetPeru
force-pushed
the
fix/gt-648-649-corpus-load-once
branch
from
July 31, 2026 23:08
5c9654d to
4032e50
Compare
beyondnetPeru
force-pushed
the
fix/gt-648-649-corpus-load-once
branch
from
August 1, 2026 00:13
7aa1e81 to
1a0de84
Compare
# Conflicts: # reference/core/control-center/maturity-reports/maturity-reconciliation.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Claims: GT-648, GT-649
Found while fixing a flaky k6 threshold in
product/infra/load/k6/smoke.js. That fix (asserting the median instead of a 10-sample p95) stops the flake gating CI and deliberately leaves the underlying defect in place — this is the defect.GT-648 — the corpus is deployment state, read once
RuleEvaluationEngine.discoverAndEvaluatecallsloadAllRulesets(corePath)on every evaluation, and core-api'sIRulesetRepositorywas a bareDiskRulesetRepositorywith no cache. EachPOST /api/v1/evaluatewalked the corpus tree, read 176*.rules.jsonfiles, and ran every one through Ajv to produce the same 393 rules as the request before it. Both halves are synchronous CPU work insideawaited calls, so nothing else on the event loop advanced while they ran.CI run 30631939687 (Reliability workflow) shows both halves: the load-time WARN block repeats once per k6 iteration, and a
GET /healththat landed during that work took 498.5 ms end-to-end —http_req_waiting498.4 ms againsthttp_req_connecting0.17 ms — while core-api logged its own handler atdurationMs=0.The cache is a decorator (
CachingRulesetRepository), not a field on the disk repository: "load once" is a property of this deployment, and the CLI is a one-shot process that must keep re-reading disk. Three properties are load-bearing and each is pinned — the in-flight promise is memoized (concurrent cold requests don't each start a walk), failures are never cached (GT-474 keeps its meaning; a repaired corpus loads on the next attempt), and each caller gets its own array.RulesetCorpusWarmupServicepreloads atOnApplicationBootstrapso request #1 — the one readiness lets through — doesn't pay it either.Also per request and compounding it:
SdlcDataLoaderService.loadAllGatesre-read the phase index once per phase. Memoized.GT-649 — the WARNs, one of which is not log noise
reference/governance/sdlc/phases/has not existed since 2026-07-04.f0d01911("remove leftover old directories under reference/") deleted the wholereference/governance/tree; GT-461 later re-createdgates/at that same path and notphases/. BecauseloadGatesForPhaseresolves gates through the phase index, all five authored gate files have been unreachable for four weeks —SatelliteEvaluationPipeline's phase-gate stage produced zero results, reported no error, and the top-level verdict was computed without it.The three "broken" rulesets are different document kinds sharing the
*.rules.jsonsuffix (an ADR-0104 recommendation catalogue with its own consumer; two single-rule declarations enforced by guard 29 and their Rego). The loader now classifies by declared$schemaand validates each against that contract — strictly more checking than before, since the old path checked the wrong schema and threw the answer away. Both failure directions still warn, so silencing did not become blindness. All three also failed the schema they themselves declare; fixed in both directions.NestLoggerProviderforwarded its optionalcontextunconditionally, and Nest reads...optionalParamspositionally — 44WARN undefinedlines across an 11-request run.Evidence
Baseline reproduced against
origin/develop@3051e499before changing anything — 11 evaluations produced exactly 11 corpus loads:Skipping non-standard ruleset(11 reqs)Phases directory not found(11 reqs)WARN undefined(11 reqs)loadAllRulesetscall/health5 ms into an in-flight evaluateVerdicts unchanged, verified not argued. The full
EvaluationResultfor the k6 smoke payload, captured from a build oforigin/developand from this change with the GT-649 phase data withheld, is byte-for-byte identical at 442,204 bytes. The normalized corpus was diffed against the pre-change loader separately: 393 rules, identical JSON.Tests: 1,975 pass across infra-providers (164), core-api (181), core-domain (1,630) and CLI (1,456 — after building the workspace dists it needs). Governance gate green, 17/17.
Three things deliberately left open
/api/v1/evaluatenow returnsgate-f1alongsidegeneral-rulesets; a satellite passing because its F1 gate never ran can now legitimately FAIL. The smoke payload's top-level verdict is unchanged (FAIL either way) but does not exercise the flip and must not be read as evidence that nothing does.core-apiandcliinfrastructure/providers/logger.provider.ts); both surfaces resolve the real one from@beyondnet/evolith-infra-providers.Notes
origin/main..origin/developand the open PRs at the time (max GT-645). Two parallel sessions then took both: GT-646: a p95 over ten samples is the max, not a percentile #327 (the k6 p95 fix, now merged) registered GT-646, and GT-647: an image must ship the whole workspace closure it will require #328 claimed GT-647 an hour before this PR opened. Renumbered to GT-648/GT-649, which guard 49 confirms are free with 0 collisions. Supersedes the closed perf(core-api): load the ruleset corpus once, and make the corpus load clean (GT-646, GT-647) #330.infra-providers/package.jsonhas declared@beyondnet/evolith-contractswhile the lockfile did not record it — theCannot find module '@beyondnet/evolith-contracts/ingest'that keeps surfacing in CI.product/infra/load/README.mddeliberately untouched: finding docs: add API dotnet standard #5 lives in the sibling k6 branch and editing that section here would collide.🤖 Generated with Claude Code