From b222992be35a1bb9062fe48477794c4559a6865d Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya Date: Mon, 10 Aug 2026 03:38:53 -0700 Subject: [PATCH] docs: cite the conformance test that exists, and record three rules where they are looked up - `check.ts` and `check.test.ts` both pointed at `tests/scripts/checks_conformance.test.mjs`, which is not a file in this repo. The generated checks are compared against a second reader of the spec by `frontend/ui-core/src/data/conformance.test.ts`; both comments now name it. - `refactor-protocol` gains the two cleanup-phase quirks whose shared property is that the command reports failure while having succeeded, and the test-double fidelity rule. - `DESIGN.md` gains the indeterminate-progress rule, which lived only in a screen comment. --- .../skills/process/refactor-protocol/SKILL.md | 20 +++++++++++++++++++ DESIGN.md | 7 +++++++ frontend/ui-core/src/data/check.test.ts | 8 ++++---- frontend/ui-core/src/data/check.ts | 7 ++++--- 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/.agents/skills/process/refactor-protocol/SKILL.md b/.agents/skills/process/refactor-protocol/SKILL.md index 9074979d..930c6dac 100644 --- a/.agents/skills/process/refactor-protocol/SKILL.md +++ b/.agents/skills/process/refactor-protocol/SKILL.md @@ -51,6 +51,7 @@ All work in the worktree; never the primary checkout. Conventional commits in lo - **`git add` new files before trusting any local check run.** Several gates read `git ls-files` — the index, not the working tree — so an untracked new file is invisible to them and passes locally while failing in CI. — 2026-08 run, T4 - **After a rebase or merge that brings in commits you did not write, lint the *whole tree*** — `ruff check .`, not the files you touched. A rename is a whole-tree fact: your branch renames a symbol, somebody else's branch adds a *new* use of the old name, and git merges both without a conflict because they are different lines. Re-running the tests you edited proves nothing either when the surviving use is a type annotation, which is never evaluated. #339 renamed a test double; #281 landed mid-flight with a fixture annotated on the old name; every targeted pytest module passed and CI answered `F821`. — #339 - **A test double must not encode invisible-order or frozen-state semantics.** Put defaults in the *unmatched-request fallback* so an explicit stub always wins whichever order it was registered in, and derive stub responses from the state the test walks rather than from frozen literals. Both failure modes make a test assert against the fixture instead of the code, and both are silent. — 2026-08 run, T6/T7/T10 +- **A test double is constructed against the real signature it doubles, and an absence assertion requires its positive path proven in the same test file.** The two halves are one rule because they fail together: a double built from a remembered signature does not run the code under test at all, and the assertion that then passes is almost always an absence — *nothing was written*, *no download started*, *the field did not change* — which a double that raises on entry satisfies vacuously. So: read the real callable or model before writing the fake (field names included — a fake `IntegrityReport` spelled one field differently made a check that never ran look like a check that found nothing), and never let a "nothing happened" assertion stand alone. Somewhere in the same file, the same double must be shown making something happen; if no test in the file exercises the positive path, the absence proves the fake is broken and not that the code is right. — #491, #496 - **A new rule is verified by breaking it — and the harness that breaks it lies in three ways unless you hold it to these.** A test that passed the moment it was written has not been shown to fail; deliberately violating the rule it guards is the only thing that tells a test from a description. Every one of the three below has already cost a run: - **Commit the work before the first mutation.** To a directory-wide revert, your uncommitted implementation and the mutation are the same edit. `git checkout -- frontend` after one mutation reverted ~20 files of finished work — and left the mutation in place, because it lived in a file git was not yet tracking. Three more then stacked on that same file and the next run came back as unrelated-looking red spread across the suite, which reads as a broken implementation rather than as a broken harness. @@ -112,6 +113,25 @@ git fetch --prune If not merged at session end: leave the worktree, report path + branch + PR URL + CI status. +**Neither of the two commands above reports its own success honestly, and both lie in the +direction of "something went wrong" when nothing did.** Confirm the state, never the exit +code — a cleanup phase re-run against an already-clean remote is how a session invents work +for itself at four in the morning. + +- **`gh pr merge` run from a worktree can exit non-zero while the merge and the branch + deletion both completed.** It squashes, deletes the remote branch, and then tries to check + out `main` locally to fast-forward it — which fails with + `fatal: 'main' is already used by worktree at …`, because the primary checkout holds it. + The exit code belongs to that last step and says nothing about the merge. Verify by SHA: + `gh pr view --json state,mergedAt,mergeCommit`. Do not re-run the merge. +- **`git ls-remote --heads` can race GitHub's branch deletion**, which is asynchronous and + lands a few seconds after the API call returns. A branch still listed immediately after a + `--delete-branch` merge is usually not a branch that survived. Sleep a few seconds and ask + again before concluding that manual cleanup is needed — and if it *is* still there on the + second reading, delete it explicitly rather than assuming the merge was partial. + +— 2026-08 inference line + ### Background processes you spawned A worktree is not the only thing a session leaves behind. Synthetic load generators, a diff --git a/DESIGN.md b/DESIGN.md index f0ff5120..fa8a3fa4 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -542,6 +542,13 @@ which leaves every one-line select on the contract's 36px. **Nothing truncates** identifier cut off in the middle is not an identifier, so a long one wraps. The specimen is on the styleguide page. +**Indeterminate progress renders as prose, never as a bar** (#494, 2026-08-10). `Progress` +draws an indeterminate value as an empty track, and an empty track reads as *0%* — a lie in +the one case where the truth is *this is going, and nobody can say how far*. So a run whose +total is unknown gets the sentence and no bar at all; the bar appears when there is a +fraction to draw. Giving the primitive an indeterminate animation would change this rule, +which makes it a design decision and not a screen's to take in passing. + ### Lists and filtering Any list that can exceed ~20 rows carries a filter input. Filtering is client-side and diff --git a/frontend/ui-core/src/data/check.test.ts b/frontend/ui-core/src/data/check.test.ts index ec0eb37b..67e354a8 100644 --- a/frontend/ui-core/src/data/check.test.ts +++ b/frontend/ui-core/src/data/check.test.ts @@ -2,10 +2,10 @@ * The combinators, one case per branch the generator can emit. * * Every fixture here is hand-written and every expectation is stated by a person. - * That is the whole design of this file: `tests/scripts/checks_conformance.test.mjs` - * compares the generated checks against a second reader of the same spec, which is a - * strong test of *agreement* and no test at all of whether either one is right. This - * file is where "right" is asserted, by someone who read the contract. + * That is the whole design of this file: `./conformance.test.ts` compares the generated + * checks against a second reader of the same spec, which is a strong test of *agreement* + * and no test at all of whether either one is right. This file is where "right" is + * asserted, by someone who read the contract. * * Each `it` fails if its branch is deleted from `check.ts`, which is the mutation * claim stated as a checklist rather than run as a tool. diff --git a/frontend/ui-core/src/data/check.ts b/frontend/ui-core/src/data/check.ts index b8f17b8e..5fbf5165 100644 --- a/frontend/ui-core/src/data/check.ts +++ b/frontend/ui-core/src/data/check.ts @@ -23,15 +23,16 @@ * widens a `const` fails the build. Four of the six ways the generator can be wrong * are caught before any test runs. The two it cannot catch — a dropped nullable and * an over-strict extra field — are both *over*-validation, and they are what - * `tests/scripts/checks_conformance.test.mjs` exists for. + * `./conformance.test.ts` exists for. * * ## Three rules, each of which is a decision * * **Unknown keys are allowed.** `additionalProperties: false` in the spec constrains * what pydantic *accepts*, not what the server may one day *send*. A client that * refused a response because it grew a field would turn every additive, backward- - * compatible API change into a broken page. Forward compatibility wins, and a test - * pins it so it cannot be tightened by accident. + * compatible API change into a broken page. Forward compatibility wins, and two tests + * pin it so it cannot be tightened by accident — `./check.test.ts` on the combinators + * and `./conformance.test.ts` across every generated check. * * **`format` is not validated.** `uuid` and `date-time` are checked as `string` and * nothing more. A renderer is protected by the *type*; rejecting a legal ISO-8601