Skip to content

Commit be72dd5

Browse files
committed
docs(observability-map): split the README and cut its history
The README carried four jobs at once after the inline commentary moved into it: what the tool measures, how to read the report, how the scanner decides, and a running account of what earlier rounds got wrong. Split it. README.md keeps the reader-facing job (running it, CI, what the score means, the five checks, headline figures, not-applicable, suppression, known limits, layout); INTERNALS.md takes the scanner reasoning, the mutation harness, the reporting arithmetic and the CI wiring, for someone changing the tool. Cut the history throughout. What a fix changed and why the old shape was broken is in the commits and the ledger; a rejected alternative is kept only where someone would otherwise re-propose it, and then in one line. Every inline 'README, <section>' pointer is repointed and every one resolves to a heading that exists. 964 lines becomes 336 + 494. The real-tree report is byte-identical.
1 parent 70a6bf8 commit be72dd5

16 files changed

Lines changed: 715 additions & 842 deletions

internal-packages/observability-map/INTERNALS.md

Lines changed: 494 additions & 0 deletions
Large diffs are not rendered by default.

internal-packages/observability-map/README.md

Lines changed: 180 additions & 808 deletions
Large diffs are not rendered by default.

internal-packages/observability-map/src/checks/authBoundary.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ const ID = "auth-boundary";
1111
* A guard the route only imports and never calls does not count, and neither does one the OTHER
1212
* export calls.
1313
*
14-
* Names rather than the three patterns it replaces, all of which over-matched: README, "Sensitivity,
15-
* and the names the tool matches on". `webappSymbols.test.ts` fails if a name stops resolving, but
14+
* Names rather than the three patterns it replaces, all of which over-matched: INTERNALS.md,
15+
* "Sensitivity, and the names the tool matches on". `webappSymbols.test.ts` fails if a name stops
16+
* resolving, but
1617
* cannot check that a declaration with the right name is the guard we meant, which is the residual
1718
* the two local helpers below carry.
1819
*/

internal-packages/observability-map/src/checks/authScope.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ function builderExports(ep: EntryPoint): BuilderExport[] {
2828

2929
/**
3030
* Whether a route the builder authenticated is also narrowed to the caller. The IDOR class it
31-
* measures, the two ways to be scoped, and why `ability.can(...)` is deliberately not a third:
32-
* README, "Authenticated is not the same as scoped".
31+
* measures: README, "The five checks". The two ways to be scoped, and why `ability.can(...)` is
32+
* deliberately not a third: INTERNALS.md, "What auth-scope reads as scoping".
3333
*
3434
* There is no triviality test here because `isTrivial` answers false for any route with an
3535
* initializer callee, and no delegating test because `scoreEntry` answers for every check before any

internal-packages/observability-map/src/checks/errorClassification.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export const BUILDERS = new Set([
3131
* An absolute count and not a ratio against the enclosing body, or padding the body relabels the
3232
* same broad swallow as a narrow guard (`inert-statements-after-try`). The count is NOT unpaddable,
3333
* which an earlier docstring claimed: it is one condition of three and no longer the load-bearing
34-
* one. See README, "Parse guards, and the narrow-try count".
34+
* one. See INTERNALS.md, "Parse guards, and the narrow-try count".
3535
*/
3636
const NARROW_TRY_STATEMENTS = 2;
3737

@@ -88,7 +88,7 @@ function swallows(clause: CatchEvidence): boolean {
8888
* and is defeated by one inert call, so `try { String(0); }` reads as classification and takes the
8989
* tree from 19 to 44. `dead-classifying-try-with-call` in the mutation corpus is that shape,
9090
* running as an expected failure. Read the rule as "refuses `try { 0; }`", never as "an unreachable
91-
* catch cannot be credited". Everything else here: README, "The dead-code defence".
91+
* catch cannot be credited". Everything else here: INTERNALS.md, "The dead-code defence".
9292
*/
9393
export const errorClassification = {
9494
id: ID,

internal-packages/observability-map/src/checks/requestContext.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ function logLevel(callee: string): string {
3232
* is the whole point rather than an oversight: its failures go to the global handler, which names no
3333
* tenant. Passing those routes, as this check used to, meant deleting every catch clause in the tree
3434
* scored it 100, and excusing them as not-applicable is the same mistake in quieter clothes. What the
35-
* platform attaches centrally and why none of it is a tenant: README, "What 19 means".
35+
* platform attaches centrally and why none of it is a tenant: README, "What the score means".
3636
*/
3737
export const requestContext = {
3838
id: ID,

internal-packages/observability-map/src/docstringReferences.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { MUTATIONS } from "./mutations.js";
77
/**
88
* Every test name a docstring in `src/` claims to be covered by must exist. The rule was asked for six
99
* times in prose and broken six times, so prose does not enforce itself. Exactly what is and is not
10-
* checked, and the coverage holes that leaves: README, "Tests, timeouts and CI".
10+
* checked, and the coverage holes that leaves: INTERNALS.md, "Tests, timeouts and CI".
1111
*/
1212

1313
const SRC = resolve(__dirname);

internal-packages/observability-map/src/integration.test.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { SCORED_CHECK_IDS } from "./checks/index.js";
2121
* The coupling is acceptable because nothing here names a route or a count: the scan must not crash,
2222
* the entry point count must sit inside a wide band, and parse failures must be zero. Those are the
2323
* only things a fixture tree cannot tell us, since a fixture only contains shapes somebody thought to
24-
* write down. How the whole suite is gated in CI: README, "Tests, timeouts and CI".
24+
* write down. How the whole suite is gated in CI: INTERNALS.md, "Tests, timeouts and CI".
2525
*/
2626
const ROUTES = resolve(__dirname, "../../../apps/webapp/app/routes");
2727

@@ -93,7 +93,8 @@ const steps = (block: string) => block.split(/^ {6}- name: /m).slice(1);
9393

9494
/**
9595
* The one thing the docstring checker cannot reach, since it walks `src/` only. What the C1 defect was
96-
* and what replaced it: README, "Tests, timeouts and CI". These are text checks over the workflow
96+
* and what replaced it: INTERNALS.md, "Tests, timeouts and CI". These are text checks over the
97+
* workflow
9798
* rather than a parse of its semantics, so they catch the wiring coming apart and nothing about
9899
* whether GitHub agrees.
99100
*/
@@ -176,7 +177,7 @@ describe("the report workflow reconciles a comment the paths no longer reach", (
176177

177178
/**
178179
* Asserts the shape that cannot have the stdout-capture bug rather than the pnpm version that happens
179-
* not to. Why, and why the render step is left alone: README, "Tests, timeouts and CI".
180+
* not to. Why, and why the render step is left alone: INTERNALS.md, "Tests, timeouts and CI".
180181
*/
181182
describe("the report workflow's two scan steps", () => {
182183
it("let the scanner write its own report rather than capturing stdout", () => {
@@ -288,7 +289,7 @@ describe("the package's tests are wired into the gate", () => {
288289
/**
289290
* The third road into this suite, `turbo run test`. Asserts the task is uncacheable, because the
290291
* config is one line and reads like a performance oversight to anyone who does not know what the suite
291-
* reads. Measurement and the rejected `inputs` alternative: README, "Tests, timeouts and CI".
292+
* reads. Measurement and the rejected `inputs` alternative: INTERNALS.md, "Tests, timeouts and CI".
292293
*/
293294
describe("the third road in, turbo", () => {
294295
it("keeps its test task out of the turbo cache", () => {
@@ -322,7 +323,7 @@ describe("counting candidates independently of the scanner", () => {
322323
* Timeout for the two real-tree tests, which do not fit the suite's 10s default. A hang detector and
323324
* nothing else: neither test asserts anything about how long a scan takes, so a number tight enough to
324325
* be a performance budget would only be a way to fail on a busy runner. The contention measurements
325-
* behind 120s, and why 60s is not enough: README, "Tests, timeouts and CI".
326+
* behind 120s, and why 60s is not enough: INTERNALS.md, "Tests, timeouts and CI".
326327
*/
327328
const TREE_SCAN_TIMEOUT = 120_000;
328329

internal-packages/observability-map/src/mutationCorpus.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const ENABLED = process.env.OBS_MAP_MUTATION_CORPUS === "1";
2323
/**
2424
* Where a corpus entry goes when the tool does not defend it. `it.fails` keeps the entry running, so
2525
* closing the hole later turns this file red until the entry is moved back out deliberately. Both
26-
* gaps are described at length in README, "The mutation harness".
26+
* gaps are described at length in INTERNALS.md, "The mutation harness".
2727
*/
2828
const KNOWN_GAPS = new Set<string>([
2929
// `canRaise` accepts any call at all, so `try { String(0); }` reads as a clause guarding real work
@@ -201,7 +201,8 @@ function mutate(
201201
* Deliberately NOT gated behind `OBS_MAP_MUTATION_CORPUS`, unlike everything below it: the corpus
202202
* cannot catch its own omission by failing, since omitting a check from the sweep lowers the score
203203
* rather than raising it. Belongs in the default suite so adding a check without extending the corpus
204-
* turns `pnpm test` red rather than a job nobody runs locally. See README, "The mutation harness".
204+
* turns `pnpm test` red rather than a job nobody runs locally. See INTERNALS.md, "The mutation
205+
* harness".
205206
*/
206207
describe("the corpus keeps up with the check registry", () => {
207208
it("suppresses every registered check in the exhaustive sweep", () => {
@@ -303,7 +304,7 @@ describeCorpus("mutation corpus over the real route tree", { timeout: ENTRY_TIME
303304
/**
304305
* How much a mutation must reach before its result means anything, since one that silently matched
305306
* nothing would otherwise pass by leaving the tree alone. On sites and not only files, and why
306-
* verdict movement cannot be the guard instead: README, "The mutation harness".
307+
* verdict movement cannot be the guard instead: INTERNALS.md, "The mutation harness".
307308
*/
308309
const MINIMUM_FILES_TOUCHED = 20;
309310
const MINIMUM_SITES_TOUCHED = 40;

internal-packages/observability-map/src/mutations.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import ts from "typescript";
33
/**
44
* Source-to-source mutations for the tree-scale corpus in `mutationCorpus.test.ts`. Why they are text
55
* rewrites rather than reprints, what `preserving` and `deleting` mean, and why the additive direction
6-
* is tracked separately in `ADDITIVE_IDS`: README, "The mutation harness".
6+
* is tracked separately in `ADDITIVE_IDS`: INTERNALS.md, "The mutation harness".
77
*/
88

99
export type MutationKind = "preserving" | "deleting";
@@ -306,7 +306,7 @@ function bindingNameOf(clause: ts.CatchClause): string | null {
306306
/**
307307
* Splice a statement in at the HEAD of every catch clause that names its binding, which is the whole
308308
* point of the helper: 234 of the tree's 260 clauses end in a `return` or a `throw`, so an appended
309-
* shape was dead by ordering before the rule under test looked at it. See README, "The mutation
309+
* shape was dead by ordering before the rule under test looked at it. See INTERNALS.md, "The mutation
310310
* harness".
311311
*/
312312
function prependToEveryCatch(

0 commit comments

Comments
 (0)