Prove the tests catch real mistakes, and tell the admin stories in plain English - #2011
Conversation
The branch mutation gate found the moved suites never pinned the form labels and hints, the flash wordings, the redirect targets, the activity-log entries, or the ordered-collection append. New direct tests assert each of those exactly: the api-keys name box and its bound, the key-only flash still showing the copy panel, the delete flash and target, the active nav entry, and a new attributes CRUD suite covering create, reorder, option add, edit, and both deletes with their logged messages. Two provably-equivalent mutants are recorded: the entity load's `?? null` after find(), and the option insert's placeholder sortOrder that append always overwrites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
📝 WalkthroughWalkthroughTransactional admin creation now shares database, ordering, and activity-log transactions. Question answer routes are separated, mutation registries support multiple files, English form copy uses sentence case, and extensive API-key, admin, servicing, i18n, and UI coverage is added. ChangesAdmin workflows and contracts
Mutation registry and copy-back
Feature and workflow coverage
Copy and localization
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fd1e906a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # resolve. | ||
| src/shared/db/news-posts.ts:148:47 news_post → "" # dropping the alias leaves the column names unqualified, and the read selects from one table (news_posts AS news_post) plus scalar subqueries that carry their own qualified id, so every name still resolves to the same column | ||
| src/features/admin/api-keys.ts:147:75 ?? → || # entity-page load: find() returns a key row object or undefined — a row object is always truthy, so `?? null` and `|| null` agree on every input | ||
| src/features/admin/attributes.ts:188:20 0 → 1 # option insert's sortOrder is a placeholder: attributeOptionsOrder.append immediately overwrites it with MAX(sibling)+1 (or the collection's start), and nothing reads the row in between — no input can see the inserted value |
There was a problem hiding this comment.
When attributeOptionsOrder.append() rejects after the separate insert has committed—for example, during a transient libsql write failure—the option remains stored with the placeholder sortOrder. The original therefore leaves 0, while this mutant leaves 1, and later reads can observe that difference, so it is not equivalent. Remove this suppression and make the insert plus ordering update atomic, with a regression test for the failed-update case.
AGENTS.md reference: AGENTS.md:L84-L84
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
You're right — a failed append could observe the placeholder, so it was never equivalent. Fixed in 6a6fe10: the option create (and the attribute create, which had the same shape) now runs its insert, order append, and activity log in one withTransaction via writeTableRow, the suppression record is removed, and two regression tests force the order write to fail with a RAISE trigger and expect nothing stored. Both tests fail against the old non-atomic code — verified by stashing the source change and watching them go red.
Generated by Claude Code
| }); | ||
|
|
||
| describe("admin UI", () => { | ||
| test("serves the name box with its label, hint, bound, and required flag", async () => { |
There was a problem hiding this comment.
This addition leaves api-keys.test.ts at 790 lines, nearly twice the repository's target for test files. Because every API-key mutation run must execute this whole mirrored suite, continuing to collect database, UI, and authentication cases here makes the mutation gate unnecessarily slow; move these new admin-UI cases into a focused file under test/features/admin/api-keys/ and split the remaining concerns while touching the suite.
AGENTS.md reference: AGENTS.md:L82-L82
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 6a6fe10 — the suite is now test/features/admin/api-keys/ with four focused files: records (database operations), ui (page, form, flash, create), manage (per-key pages and the delete flow), and auth (Bearer + JSON API). Largest is 258 lines. The local POST scaffold was replaced with the shared adminFormPost, and the repeated flash-page GETs folded into one helper.
Generated by Claude Code
| }); | ||
|
|
||
| describe("POST /admin/attributes", () => { | ||
| test("creating an attribute stores it, logs it, and says so", async () => { |
There was a problem hiding this comment.
The new direct suite recreates CRUD journeys that remain in test/integration/server/attributes.test.ts:94-348, including create and validation, editing, option creation and ordering, and both confirmed deletes. These mirrored tests are needed for mutation selection, but copying rather than moving and extending the existing cases makes every flow run twice and leaves two expectation sets that can drift; consolidate each journey in this direct suite and remove its integration duplicate.
AGENTS.md reference: AGENTS.md:L73-L73
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 6a6fe10 — the whole "attribute CRUD" describe is gone from the integration file: its cases merged into the mirror suites (crud.test.ts for attribute-level, a new options.test.ts for option-level), each journey now stated once with the killer assertions folded in. The integration file keeps only the non-CRUD pages (index, listing attributes tab).
Generated by Claude Code
… atomic Three review points, all done: The attribute CRUD journeys now live only in the mirror suites — the new crud and options files under test/features/admin/attributes/ merge the killer assertions with the moved integration cases, and the integration file keeps just the pages that are not CRUD. The api-keys suite is now a folder of four focused files (records, ui, manage, auth), each under the size target, sharing adminFormPost and a page helper instead of local scaffolds. The option and attribute creates now run their insert, order append, and activity log in one transaction. The "equivalent" record for the placeholder sortOrder was wrong — a failed append used to leave the half-made row behind — so it is removed, and two regression tests force the order write to fail and expect nothing to be stored. Both fail against the old non-atomic code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The join form's labels, hints, bounds, and autocomplete are now pinned by a render test. The i18n loading layer gets direct tests for its guards and bookkeeping: a non-string message and a key claimed by two groups both refuse to load, a group loads once even when ensured twice or concurrently, a failed load can be tried again, and withMessageGroups loads copy before the work runs. The rebrand scanner gets three more stories: the closing bracket of a split tag is markup, and copy after a plural adopts the first branch's exit state. Four provably-equivalent scanner mutants are recorded (unread residual tagIsCode writes, and the inert "<" scan start). The api-keys page pins its input name, the exact flash shape of a created key, and which flash slot renders a key-only message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The listing attributes tab save gets its own direct suite: the chosen options are stored, the activity line and flash say so, and the route 404s while the feature is off. The factory now hands readIds the configured field name, so option_ids has one source of truth instead of a dead config field beside a hard-coded copy. Option edits assert their logged message and moves their flash. The database-reset form pins its label and autocomplete, and a wrong phrase goes back to the reset page itself. The transactional option insert's placeholder sortOrder is recorded as provably equivalent — the same transaction always overwrites or removes it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The home and contact form definitions were unasserted: the boxes now pin their names, labels, hints, autocomplete, placeholder, and the markdown preview wiring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/features/admin/api-keys/auth.test.ts`:
- Around line 182-200: Move the tests named “touchApiKeyLastUsed surfaces the
test-override error to its caller” and “getApiKeyForUser throws a not-found
error for an unknown key” from the admin JSON API suite into records.test.ts.
Add their dependencies to that file’s top-level static imports, remove the
inline dynamic imports, and preserve both assertions and the override cleanup
behavior.
In `@test/features/admin/api-keys/manage.test.ts`:
- Around line 17-42: Split the combined test around the never-used placeholder
and last-used date into separate tests, preserving the shared setup needed by
each behavior. In the used-key test, assert that the second response has status
200 before inspecting its body, and use stronger exact assertions for the
expected rendered date rather than only checking that “Never” is absent.
In `@test/features/admin/api-keys/records.test.ts`:
- Around line 132-136: Update the “getApiKeyForUser throws for wrong user” test
to assert the exact expected error message, matching the established unknown-key
assertion in auth.test.ts, instead of using a bare rejects.toThrow().
In `@test/features/admin/api-keys/ui.test.ts`:
- Around line 51-62: Update the test named “serves the name box with its label,
hint, bound, and required flag” to extract the actual input tag containing
name="name" instead of slicing from a fixed negative-prone offset. Keep the
existing maxlength, placeholder, and required assertions scoped to that
extracted tag.
In `@test/features/admin/attributes/crud.test.ts`:
- Around line 113-131: The trigger setup and cleanup are duplicated across both
rollback tests. Add a shared withFailingOrderTrigger helper that accepts the
table name and callback, creates the failing sort_order trigger, executes the
callback, and always drops the trigger; replace the inline boilerplate in
test/features/admin/attributes/crud.test.ts lines 113-131 with the helper for
attributes, and in test/features/admin/attributes/options.test.ts lines 58-82
with the helper for attribute_options.
- Around line 220-231: Update the delete-confirmation assertion in the admin
attribute deletion test to match the complete expected message exactly:
“Attribute name does not match. Please type the exact attribute name to confirm
deletion.” Replace the partial expect.stringContaining check while preserving
the existing response and attribute-preservation assertions.
In `@test/features/admin/database-reset.test.ts`:
- Around line 143-145: Update the redirect assertion in the database reset test
to parse the Location header as a URL and compare its pathname exactly to
"/demo/reset". Continue allowing expected query parameters without requiring the
full URL to match.
In `@test/features/admin/site.test.ts`:
- Around line 29-55: Scope the assertions to the exact controls: in
test/features/admin/site.test.ts lines 29-55, extract each named field from
siteHomeForm and siteContactForm before asserting its labels, hints,
placeholders, autocomplete, and markdown-preview metadata; in
test/features/join.test.ts lines 26-44, replace the broad 300-character HTML
slices with field-specific extraction and assertions for the password and
password_confirm controls.
In `@test/test-utils/db-helpers/attributes.ts`:
- Around line 51-60: Replace the function-scoped dynamic imports in
createAttributeViaRoute with top-level static imports for adminFormPost from the
session utility and expectRedirect from the assertions utility. Keep the
function’s request, redirect validation, and ID extraction behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c9cebe0f-24e4-42b7-acb8-eef6a172d5a6
📒 Files selected for processing (19)
scripts/mutation/equivalent-mutants.txtsrc/features/admin/attributes.tssrc/features/admin/listing-choice-post.tstest/features/admin/api-keys.test.tstest/features/admin/api-keys/auth.test.tstest/features/admin/api-keys/manage.test.tstest/features/admin/api-keys/records.test.tstest/features/admin/api-keys/ui.test.tstest/features/admin/attributes/crud.test.tstest/features/admin/attributes/listing-tab.test.tstest/features/admin/attributes/options.test.tstest/features/admin/database-reset.test.tstest/features/admin/site.test.tstest/features/join.test.tstest/integration/server/attributes.test.tstest/shared/i18n.test.tstest/shared/rebrand.test.tstest/test-utils/activity-log.tstest/test-utils/db-helpers/attributes.ts
💤 Files with no reviewable changes (1)
- test/features/admin/api-keys.test.ts
CodeRabbit's nine points, all done: the record-layer tests moved into records with static imports, the manage-page test split with statuses asserted, the wrong-user error pinned exactly, form assertions scoped to the extracted control (inputNamed now matches textareas and selects too), the rollback boilerplate folded into withFailingOrderTrigger, the attribute mismatch message asserted in full, the reset redirect compared by pathname, and the fixture helper's imports made static. The three insert-and-order flows (attribute, option, answer) now share one mechanism: insertScopedOrderedRow in ordered-collection.ts rides writeRowInTransaction, so a failed order write or log line rolls the insert back — the question and answer creates were still non-atomic. The stale question ignore-list records (the deleted form id) are gone, and new tests pin the question and answer forms, the display-as choices, the delete flow's exact messages, and the ledger entry form's required flags and zero floor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
Same rule the review applied next door: no cycle exists, so the function-scoped import was only indirection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The helper now hands back any named control, so its miss message says "control" — and the test asserts the textarea and select cases too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbd10257f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| test("serves the home form boxes with their labels and hints", async () => { | ||
| const { siteHomeForm } = await import("#routes/admin/site.ts"); | ||
| const html = siteHomeForm.render(); | ||
| expect(html).toContain("Website Title"); |
There was a problem hiding this comment.
This new assertion pins the legacy Website Title wording, while the same added block also pins Homepage Text and Contact Page Text. Because this change directly touches that form surface, update the catalog/form copy and expectations to sentence case (Website title, Homepage text, and Contact page text) rather than adding regression tests that preserve the old Title Case labels.
AGENTS.md reference: AGENTS.md:L231-L234
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 52910d3 — the catalog copy is now sentence case (Website title, Homepage text, Contact page text) and the form tests pin the new wording, so the old Title Case labels would fail the assertions rather than being preserved by them.
Generated by Claude Code
| }; | ||
|
|
||
| describeWithEnv("server (admin site)", { db: true }, () => { | ||
| describe("the site forms", () => { |
There was a problem hiding this comment.
Adding this describe block leaves test/features/admin/site.test.ts at 560 lines, well above the repository's roughly 400-line target for tests. These form-schema assertions are independent of the broader route suite, so keeping them here makes mutation runs against the site feature execute an unnecessarily large file; move them into a focused site-forms test while touching the suite.
AGENTS.md reference: AGENTS.md:L82-L82
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 9528978 — site.test.ts is now a test/features/admin/site/ folder: home (the form pins plus the homepage routes), contact (routes plus the Botpoison block), order, and nav. The largest file is 180 lines, and the local redirect helper was replaced with the stronger shared expectRedirectWithFlash, which checks the exact redirect path as well as the flash message.
Generated by Claude Code
| export const insertScopedOrderedRow = | ||
| <Input>( |
There was a problem hiding this comment.
The returned closure has a Promise<number> annotation, but the exported generic insertScopedOrderedRow function itself still relies on inference for its curried function type. Give the outer function an explicit, preferably named, return type so changes to its implementation fail at the definition instead of silently changing the public contract and forcing callers to re-infer the anonymous signature.
AGENTS.md reference: AGENTS.md:L97-L97
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 52910d3 — the curried type now has a name, ScopedOrderedRowInsert<Input>, and insertScopedOrderedRow declares it as its return type, so a drift in the implementation fails at the definition instead of reshaping the public contract.
Generated by Claude Code
| const questionId = await writeRowInTransaction( | ||
| await questionsTable.insertStatement!({ displayType, text }), | ||
| null, | ||
| async (transaction, id) => { | ||
| await questionsOrder.append({ key: id, transaction }); |
There was a problem hiding this comment.
Fresh evidence beyond the resolved attribute review is that this later change also makes question and answer creation atomic, but the question suites never force questionsOrder.append() or answersOrder.append() to fail after the insert. Their existing feature-write and successful-order tests do not reproduce the half-created-row defect this code fixes, so a future loss of the transaction could pass unnoticed; add route-level failing-order tests that assert no question or answer remains.
AGENTS.md reference: AGENTS.md:L84-L84
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 52910d3 (and refined in 596ce08) — route-level failing-append tests now cover both creations: withFailingOrderTrigger forces the order write to fail after the insert, and the tests assert the POST rejects with "order write failed" and that no question (test/features/admin/questions/delete.test.ts) or answer (test/features/admin/questions/answers.test.ts) remains stored. Both fail against the pre-transaction code.
Generated by Claude Code
| // One transaction: a question must never exist without its order entry | ||
| // or its log line. | ||
| const questionId = await writeRowInTransaction( |
There was a problem hiding this comment.
This refactor grows src/features/admin/questions.ts from 567 to 583 lines, leaving the route module substantially above the repository's roughly 400-line target. The answer and question lifecycle handlers are already separable concerns, so extract a focused module instead of adding more transactional creation logic to the existing monolith.
AGENTS.md reference: AGENTS.md:L82-L82
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 52910d3 — the module is now three files: questions.ts (question lifecycle plus the route table, 250 lines), questions/answers.ts (the answer lifecycle handlers), and questions/forms.ts (the two form definitions, which also breaks the route→template init cycle). The mirror tests moved with the split in 596ce08.
Generated by Claude Code
| # rebrand scanner — four provably-equivalent survivors. | ||
| # tagIsCode is read in exactly one place: stepTag's close, reachable only while | ||
| # inTag is true — and every assignment of inTag=true (stepProse's unfinished | ||
| # tag) also assigns tagIsCode. So a stale tagIsCode value can never be read. |
There was a problem hiding this comment.
These additions take scripts/mutation/equivalent-mutants.txt from 996 to 1,008 lines, crossing the repository's hard 1,000-line ceiling. Refactor the ignore loader to consume focused registry files, or otherwise split this registry, instead of continuing to append proofs to a single oversized file.
AGENTS.md reference: AGENTS.md:L82-L82
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 9528978 — the registry is now the scripts/mutation/equivalent-mutants/ directory: nine focused per-area files (largest 236 lines) plus a README with the recording rules. loadIgnoreList merges every *.txt in the directory, the static audit checks and prunes each file in place, and cross-file duplicates fail the run just as same-file ones did. Covered by new loader and audit tests (directory merge, cross-file duplicate, per-file prune).
Generated by Claude Code
The touched site labels are sentence case now (Website title, Homepage text, Contact page text). insertScopedOrderedRow declares its curried contract as a named type. The failing-order trigger helper moved to its own db-helper and covers questions and answers too, with route-level rollback tests proving a failed order write leaves no half-made question or answer. The questions route module split: answer lifecycle handlers live in questions/answers.ts, and the two form definitions in questions/forms.ts — a leaf both route modules and the page template import, which unwinds the init cycle the first cut created. The old mirror file moved to questions/forms.test.ts (loading its message group first, as the area loader does) and the redundant form assertions in delete.test.ts are gone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
…istry - New literal schema tests for the admin and listing field builders, via a shared fieldShape test helper, so any changed name, label, hint, bound, flag, or choice fails a test instead of moving the expectation along. - test/features/admin/site.test.ts split into a site/ folder (home, contact, order, nav), each file under 400 lines; the local redirect helper replaced with the stronger shared expectRedirectWithFlash. - scripts/mutation/equivalent-mutants.txt (1,008 lines) split into a scripts/mutation/equivalent-mutants/ directory of focused per-area files; the loader merges every *.txt in the directory and the static audit prunes each file in place. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The answer edit suite moves under questions/answers/ and the failed-append rollback test for answers moves into answers.test.ts, so mutants in src/features/admin/questions/answers.ts select them as direct tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The mutation run on the listing fields surfaced six survivors: inputmode, pattern, title, and defaultValue were not part of the asserted field shape, and no test rendered the builder or name-autofocus views. fieldShape now carries those keys, the literals state them, and two new tests pin what each view reveals. The directory-listing loop shared by the registry loader and the run-folder scan is now one namesInDirectory helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd448a9d38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let text: string; | ||
| try { | ||
| text = await Deno.readTextFile(file); | ||
| } catch { |
There was a problem hiding this comment.
With the new multi-file registry, if one shard exists but cannot be read because of a permission or other I/O error, this blanket catch skips it and returns a partial ignore set, making the mutation run behave as though those records do not exist instead of exposing the filesystem failure. Only treat Deno.errors.NotFound as the documented empty case and rethrow every other error.
AGENTS.md reference: AGENTS.md:L138-L146
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 32c2aed — EQUIVALENT_MUTANTS_DIR_PATH is removed; the audit CLI derives its copy-back paths from listRegistryFiles(), so the directory has one source of truth.
Generated by Claude Code
There was a problem hiding this comment.
(Correction — the reply above answered the neighbouring thread.) For this one: done in 32c2aed — the catch now calls rethrowUnlessNotFound, so only a missing file reads as empty and any other I/O failure surfaces, with a regression test passing a directory as a registry file.
Generated by Claude Code
| export const EQUIVALENT_MUTANTS_DIR_PATH = | ||
| "scripts/mutation/equivalent-mutants"; |
There was a problem hiding this comment.
A repo-wide search finds no consumer of EQUIVALENT_MUTANTS_DIR_PATH; the new copy-back code derives each path from listRegistryFiles() instead. This leaves a dead internal API that can drift from the actual directory without affecting any caller, so remove the obsolete export.
AGENTS.md reference: AGENTS.md:L81-L81
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 32c2aed — the catch now calls rethrowUnlessNotFound, so only a missing file reads as the documented empty case and any other I/O failure fails the run. Regression test: a directory passed as a registry file now rejects instead of loading as empty.
Generated by Claude Code
There was a problem hiding this comment.
(Correction — the reply above answered the neighbouring thread.) For this one: done in 32c2aed — EQUIVALENT_MUTANTS_DIR_PATH is removed and the audit CLI derives its copy-back paths from listRegistryFiles().
Generated by Claude Code
| insertStatement?: (input: Input) => Promise<{ | ||
| args: InValue[]; | ||
| sql: string; | ||
| }>; |
There was a problem hiding this comment.
If this shared helper is passed one of the façade tables that legitimately omits insertStatement, the optional property lets the call type-check and the non-null assertion then turns it into a runtime TypeError before the transaction starts. Make insertStatement required in this helper's table parameter so an unsupported table is rejected at compile time instead of pretending the method exists.
AGENTS.md reference: AGENTS.md:L128-L137
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 32c2aed — defineTable now returns the new CrudTable type, which guarantees insertStatement, updateStatement, and findByIdPrimary, and insertScopedOrderedRow requires insertStatement in its parameter type, so passing a façade table without it is a compile error. The now-redundant non-null assertions on defineTable-built tables were removed with it.
Generated by Claude Code
| const LISTING_FIELDS = [ | ||
| { | ||
| hint: "Displayed to attendees on the ticket page", | ||
| label: "Listing Name", |
There was a problem hiding this comment.
This new literal schema pins legacy Title Case labels such as Listing Name, Listing Type, and the later Max Attendees, so a compliant catalog cleanup would now fail the regression suite. Because this change directly adds coverage for the listing-form surface, update the catalog and these expectations to sentence case instead of locking the old wording in place.
AGENTS.md reference: AGENTS.md:L231-L234
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 32c2aed — the listing form labels are sentence case in the catalog now (Listing name, Max attendees, Ticket price (leave empty for free), …), the derived validation strings follow (Listing type must be standard or daily, Booking duration (days) must …), and this suite plus every other test asserting those labels pins the new wording. Detail-page column headers keep their own unchanged keys.
Generated by Claude Code
| test("the holiday form serves exactly its declared fields", () => { | ||
| expect(getHolidayForm().fields.map(shape)).toEqual([ | ||
| { | ||
| label: "Holiday Name", |
There was a problem hiding this comment.
The new admin-schema suite likewise makes legacy Title Case copy such as Holiday Name, Start Date, and many later field labels part of the expected contract. Since this PR is touching these form surfaces by adding exact copy assertions, update the message catalog and expectations to sentence case rather than preserving wording the repository requires changing when next touched.
AGENTS.md reference: AGENTS.md:L231-L234
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 32c2aed — the admin form labels are sentence case in the catalog (Holiday name, Start date, Site name, Square access token, …), the hard-coded builder and logistics labels follow, and this suite plus the holiday, builder, built-sites, and logistics tests pin the new wording.
Generated by Claude Code
…tence case - The unused registry path export is gone, and a registry file that exists but cannot be read now fails the run instead of reading as empty (with a regression test). - defineTable's return type is the new CrudTable, which guarantees the transactional statement builders, so insertScopedOrderedRow demands insertStatement at compile time and the redundant non-null assertions on defineTable-built tables are gone. - The admin and listing form labels the new schema tests pinned are now sentence case in the catalog (Listing name, Holiday name, Site name, ...), matching the house style; validation messages and every test that asserts those labels follow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93c6dbae6d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Only a --write audit rewrites the list, so only it keeps a file. | ||
| copyBack: options.write ? [EQUIVALENT_MUTANTS_PATH] : [], | ||
| // Only a --write audit rewrites the registry, so only it keeps files. | ||
| copyBack: options.write ? await registryCopyBackPaths() : [], |
There was a problem hiding this comment.
When mutation:audit-equivalents --write runs while another process edits or removes a later registry shard, this now passes multiple files to bringFilesBack, which validates and writes them sequentially (snapshot-copy-back.ts:57-66). Earlier shards can therefore be replaced before the later conflict makes the run fail, leaving the live registry only partly pruned; preflight every shard against its saved before value before writing any of them.
AGENTS.md reference: AGENTS.md:L978-L982
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in b70bd07 — bringFilesBack now checks every kept file against its saved before value before writing any of them, so a mid-run edit to any shard stops the whole copy while the live registry is still untouched. Regression test: with two kept files and the second edited mid-run, the first stays unwritten.
Generated by Claude Code
Five suites asserted the old Title Case labels with shorter fragments the first sweep missed (setup, closes-at edit form, months-per-unit visibility, create-form price); they now pin the new wording, while detail-page rows keep their own unchanged column keys. The renewal-tier warning follows the renamed Months per unit label. bringFilesBack now checks every kept file before writing any, so a multi-file audit result is never left partly applied — with a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The mutation gate requires a direct suite at the mirrored path for every changed source; the factory now has one covering the saved ids, the exact plural and singular log lines, the field name handed to a custom id reader, and the feature gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 606d21815b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| { | ||
| hint: "For items people buy without being checked in, such as raffles, fundraisers, donations, merch, or digital products. Hides QR codes, check-in, and wallet passes. Shows ‘Buy now’ instead of ‘Reserve’.", | ||
| label: "No Check-In", |
There was a problem hiding this comment.
Fresh evidence beyond the resolved listing-label thread is that this new schema expectation still pins No Check-In, while the catalog retains the same wording. The listing editor therefore keeps showing a touched legacy Title Case label; change the catalog and expectation to No check-in so the regression test enforces the required copy style instead of preserving the violation.
AGENTS.md reference: AGENTS.md:L231-L234
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 27c7203 — No Check-In is now No check-in in the catalog, the admin filter label, the renewal-tier warning, the guide copy, and every test that pins the wording, including this schema literal.
Generated by Claude Code
| for (const entry of files) { | ||
| await assertUnchanged(root, entry); |
There was a problem hiding this comment.
Fresh evidence after the resolved multi-shard preflight thread is that assertUnchanged is now separated from the writes: during mutation:audit-equivalents --write, another process can edit a shard after this loop checks it but while earlier shards are copied by the following loop, and copyOneBack then overwrites that edit without rechecking. The validation must remain effective through the complete write operation, such as through an appropriate lock or a revalidation-and-rollback strategy.
AGENTS.md reference: AGENTS.md:L978-L982
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 27c7203 — bringFilesBack now rechecks each file against its saved before value at the moment it writes it, in addition to the up-front preflight, so an edit landing between the preflight and that file's turn still stops the overwrite.
Generated by Claude Code
| # Table inputs make these operators equivalent: nullable query results exclude | ||
| # undefined, paired null checks already reject both nullish values, and optional | ||
| # conditions/dependencies are objects or arrays. | ||
| src/shared/db/table.ts:445:28 ?? → || # condition?.args is an array or undefined, and every array is truthy |
There was a problem hiding this comment.
These sharded records retain coordinates from before the source refactors: for example, src/shared/db/table.ts:445:28 now points at a function parameter while the recorded ?? moved to 455:28; the other four table.ts records moved by ten lines, src/features/admin/attributes.ts:210:55 moved to 215:45, and the questions-template record moved from 267:50 to 270:50. When the audit or mutation gate covers any of these touched files, it reports a stale entry with no matching generated mutant and fails instead of suppressing the proven equivalent, so update every shifted coordinate in the new shards.
AGENTS.md reference: AGENTS.md:L975-L977
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 27c7203 — all seven shifted records now carry the coordinates the generator produces on the current tree (table.ts +10 lines, attributes.ts at 215:46, the questions template at 270:50), each verified against generateMutants output rather than eyeballed.
Generated by Claude Code
… check-in - The registry records for table.ts, attributes.ts, and the questions template moved when those files were refactored; their coordinates now match the mutants the generator produces on the current tree (verified against generateMutants). - bringFilesBack rechecks each file at the moment it writes it, so an edit landing between the preflight and that file's turn still stops the overwrite. - The last touched Title Case label, No Check-In, is now No check-in across the catalog, the filter label, the tier warning, the guide copy, and the tests that pin them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The two files whose only change was a comment revert to their old wording, so the mutation gate's changed set stays limited to real copy changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db8219e5a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| export const EQUIVALENT_MUTANTS_FILE = new URL( | ||
| "./equivalent-mutants.txt", | ||
| export const EQUIVALENT_MUTANTS_DIR = new URL( |
There was a problem hiding this comment.
A repo-wide search shows that EQUIVALENT_MUTANTS_DIR is never imported; it is only used as listRegistryFiles' internal default. Exporting it creates an unused public API that can be changed independently of the actual registry-loading mechanism, so remove the export modifier and keep the directory path private.
AGENTS.md reference: AGENTS.md:L81-L81
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in c985d99 — EQUIVALENT_MUTANTS_DIR is private again; listRegistryFiles remains the only way in.
Generated by Claude Code
| }): Response => errorRedirect(`/admin/questions/${args.params.id}`, args.error); | ||
|
|
||
| /** Handle POST /admin/questions/:id/answers (add answer) */ | ||
| export const handleAddAnswer = createAuthedFormRoute< |
There was a problem hiding this comment.
The split makes handleAddAnswer and the other handle* closures in this module exported functions, but their public signatures are inferred from factory calls. Give these exports explicit, preferably named handler types so implementation changes fail at their definitions instead of silently reshaping the contract consumed by questions.ts.
AGENTS.md reference: AGENTS.md:L97-L97
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in c985d99 — every exported handler in the module (handleAddAnswer, the four answer page/post handlers, the recalculate pair, and answerOrder) now declares its ParamsRoute<…> contract explicitly, so a drift in a factory call fails at the definition.
Generated by Claude Code
| const validateRenewalConfig = (input: ListingInput): string | null => { | ||
| if ((input.monthsPerUnit ?? 0) > 0 && !(input.purchaseOnly && input.hidden)) { | ||
| return "Months per unit requires No Check-In and Hidden to be enabled"; | ||
| return "Months per unit requires No check-in and Hidden to be enabled"; |
There was a problem hiding this comment.
When an operator sets months per unit without both required flags, this touched message still presents Hidden as a title-cased common word and omits the full stop required for an error sentence. Change it to something like Months per unit requires No check-in and hidden to be enabled. so the rewritten user-facing message follows both copy rules.
AGENTS.md reference: AGENTS.md:L231-L237
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in c985d99 — the message is now the full sentence Months per unit requires No check-in and Hidden listing to be enabled., naming both options by their real labels, with the two asserting tests updated.
Generated by Claude Code
| ]); | ||
| }); | ||
|
|
||
| test("the logistics agent form serves exactly its declared field", () => { |
There was a problem hiding this comment.
This newly added suite is 429 lines and is selected alongside the existing 478-line test/ui/templates/fields/listing.test.ts whenever src/ui/templates/fields/listing.ts is mutation-tested, so every mutant now pays for more than 900 lines of tests. Split the schema cases into focused files under test/ui/templates/fields/listing/ rather than introducing another suite above the repository's roughly 400-line target.
AGENTS.md reference: AGENTS.md:L82-L82
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in c985d99 — the field literals moved to test/ui/templates/fields/listing/expected-fields.ts (a data module, 360 lines) and the suite itself is now 78 lines of assertions, so mutants pay for the tests rather than the table.
Generated by Claude Code
Three new acceptance Features cover the user-facing rules the branch's direct tests hardened, driving the real served pages end to end: - An owner decides what buyers are asked: a question is asked only where it is assigned, a buyer's own words reach the owner's list download, a written question's page offers no choices to manage, and taking a question away needs its exact text. - An owner describes what is on offer: a detail marked on a listing is shown to that listing's visitors with the picked wording only, and removing a detail needs its exact name. - An owner writes the front pages: the homepage and contact page read back what was saved, and the order page stays off until turned on with its introduction. The shared sells-a-thing fixture now keeps the site's own thank-you page so stories can read what a customer is shown after booking. New shared step drivers (tick a box on a listing's tab, take a thing down from its own page) serve both new domains. Also from review: the registry directory constant is private again, the exported answer route handlers carry explicit ParamsRoute types, the renewal-tier error is a full sentence naming the real labels, and the listing schema literals moved to their own data module so the suite stays under the size target. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The spec-support refactor on main keeps each person's browser in the world's things registry and renames the listing lookup to listingNamed; the new question, detail, and front-page drivers now ride on those. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The audit refuses a flag-like --registry value instead of eating the next option, splitFlagValues gets its own direct suite, inputNamed requires a real control tag name before matching, the detail-table omission checks reject either casing of the old labels, the refused option writes are checked against the store, and the builder's shared auto-provision hint moves into the catalog. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
|
Answering the two outside-diff items from CodeRabbit's latest review, since they have no inline thread:
Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e915f277a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "guide.q.how_do_i_set_up_sumup": "How do I set up SumUp?", | ||
| "guide.a.how_do_i_set_up_sumup": "<p>SumUp uses its Hosted Checkout, and the webhook is handled automatically — there's nothing to configure in the SumUp dashboard. You need two values, and <strong>both must come from the same SumUp account</strong>:</p><ol><li><strong>API Key</strong> — a secret API key from your SumUp dashboard. The developer settings are easy to miss: go to <strong>Profile</strong> → <strong>Settings</strong> → <strong>For Developers</strong> → <strong>Toolkit</strong> → <strong>API Keys</strong>, or use this direct link: <a href=\"https://me.sumup.com/en-gb/settings/api-keys\">me.sumup.com/en-gb/settings/api-keys</a>. That page shows a <strong>Public API key</strong> near the top — <strong>that is not the one you need</strong>. Instead, in the <strong>API Keys</strong> section below it, click <strong>Create API key</strong>, give it a name, and copy the new (private) key — it is shown only once.</li><li><strong>Merchant Code</strong> — your SumUp merchant code (a short identifier, usually starting with <code>M</code>), shown in your SumUp dashboard under your profile/account settings.</li></ol><p>Paste both into the SumUp section on the <a href=\"/admin/settings\">Settings</a> page and save, then click <strong>Test Connection</strong> to verify they work. The test looks up your merchant account using the API key, so if the two don't belong to the same account — for example a <em>sandbox</em> API key paired with your live merchant code — it reports a <code>401 Unauthorized</code> error. If you see that, re-copy the key in full and double-check the merchant code is the one shown in the same SumUp account that created the key.</p><p>SumUp only supports certain currencies. If your site currency isn't one of them, saving the credentials is blocked with a message asking you to choose a different provider.</p>", | ||
| "guide.q.booking_duration_field": "What is the Booking Duration field?" | ||
| "guide.q.booking_duration_field": "What is the Booking duration field?" |
There was a problem hiding this comment.
Fresh evidence beyond the resolved booking-duration wording is that the payments guide now renders What is the Booking duration field?; Booking is a common word in the middle of the sentence, so this remains Title Case. Change it to What is the booking duration field? so the touched guide copy is fully sentence-cased.
AGENTS.md reference: AGENTS.md:L231-L234
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 89e0a47 — the guide question reads What is the booking duration field?; the exact-label reference (Booking duration (days)) stays in the answer body where it names the control.
Generated by Claude Code
| "fields.built_site.db_token": "Database Token", | ||
| "fields.built_site.db_token": "Database token", | ||
| "fields.built_site.db_token_placeholder": "Database auth token", | ||
| "fields.built_site.auto_provision_hint": "Leave blank to auto-provision a database", |
There was a problem hiding this comment.
When an operator opens the builder, this catalog value is shown as the complete imperative sentence Leave blank to auto-provision a database, but it has no final full stop. Add the full stop so the newly catalogued hint follows the copy rule for full sentences.
AGENTS.md reference: AGENTS.md:L235-L237
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 89e0a47 — the hint ends with a full stop, and the builder schema pin follows.
Generated by Claude Code
| "questions.linked_all_listings": "Linked {type} (all):", | ||
| "questions.save_listings": "Save Listings", | ||
| "fields.answer.times_selected": "Times Selected" | ||
| "fields.answer.times_selected": "Times Selected", |
There was a problem hiding this comment.
When an operator edits an answer, this catalog value labels the running-total field as Times Selected, leaving the common second word in Title Case on the question surface updated by this change. Rename it to Times selected so the field label follows the required sentence-case convention.
AGENTS.md reference: AGENTS.md:L231-L234
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 89e0a47 — both Times Selected values (the answer-edit field label and the matching column header) now read Times selected, with the header pin updated.
Generated by Claude Code
| world, | ||
| `/admin/listing/${listing.id}/${tab}`, | ||
| ); | ||
| await fillInAndSend(browser, { [field]: String(boxId) }, "Save"); |
There was a problem hiding this comment.
When the attributes story creates two options and the selected option's checkbox disappears while another checkbox with the same field name remains, fillInAndSend only proves that some control named option_ids exists; its checkbox path never checks that String(boxId) is one of the rendered values. The helper then posts the unrendered ID directly, so the server can save it and the new visitor-facing story can pass despite the operator having no way to select that option. Check that the exact checkbox value is present and enabled before submitting it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 89e0a47 — tickOnListingTab now calls requireCheckboxOffered before submitting, which throws unless the served page renders an enabled checkbox for that field sending exactly that value (naming the values it did offer). The helper has its own direct tests for the offered, missing, and no-boxes cases, and both stories that tick listing-tab boxes still pass.
Generated by Claude Code
tickOnListingTab now refuses a checkbox value the served page does not offer, through requireCheckboxOffered with its own direct tests. The builder hint gains its full stop, the guide question lowercases booking, and the answer totals label reads Times selected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89e0a47f65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const validateRenewalConfig = (input: ListingInput): string | null => { | ||
| if ((input.monthsPerUnit ?? 0) > 0 && !(input.purchaseOnly && input.hidden)) { | ||
| return "Months per unit requires No Check-In and Hidden to be enabled"; | ||
| return "Months per unit requires No check-in and Hidden listing to be enabled."; |
There was a problem hiding this comment.
When an operator submits months per unit without both required flags, this changed error is returned directly through validateListingInput, so configured copy replacements and later catalog updates cannot affect it. Add a catalog key and return it through t(...), as the adjacent customisable-days validation does.
AGENTS.md reference: AGENTS.md:L186-L191
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 2e81e83 — both renewal errors (error.months_per_unit_needs_flags and its sibling error.initial_site_months_required) live in the catalog and return through t(...).
Generated by Claude Code
| <> | ||
| <p> | ||
| For daily listings, <strong>Booking Duration (days)</strong> sets | ||
| For daily listings, <strong>Booking duration (days)</strong> sets |
There was a problem hiding this comment.
When an operator reads the payments guide, this edited field name bypasses the catalog even though fields.listing.duration_days already contains the same label. Render that key through t(...) so rebranding or a future wording change cannot leave the guide and listing form saying different things.
AGENTS.md reference: AGENTS.md:L186-L191
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 2e81e83 — the guide renders t("fields.listing.duration_days"), so it can never drift from the listing form's label, and the leftover allowlist ratcheted down with it.
Generated by Claude Code
| const logisticsAgentFields = [ | ||
| { | ||
| label: "Agent Name", | ||
| label: "Agent name", |
There was a problem hiding this comment.
When the logistics agent form renders, this changed inline label bypasses the existing logistics.agent_name catalog entry, creating two independently editable sources for the same user-facing text and excluding this form from configured copy replacements. Resolve the existing key through t(...) instead.
AGENTS.md reference: AGENTS.md:L186-L191
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 2e81e83 — the agent form's label resolves the existing logistics.agent_name key through t(...), and the allowlist count came down accordingly.
Generated by Claude Code
| answer: string, | ||
| ): Promise<void> => { | ||
| const browser = await visitorOpensBooking(world, listingName); | ||
| await browser.submitForm( |
There was a problem hiding this comment.
If the booking page stops rendering one of these fields, disables it, or fixes it to another value, TestBrowser.submitForm still appends every caller-supplied value and the scenario can create a booking that no browser could submit. questionFieldOn only checks for a matching substring and does not validate the other fields, so pass the values through fillInAndSend before submission.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 2e81e83 — the booking goes through fillInAndSend, so every field (email, name, quantity, and the answer box) must be one the served page really renders, enabled, and able to carry the value before anything is sent.
Generated by Claude Code
| if (answerColumn === -1) { | ||
| throw new Error(`The list download has no column for "${question.text}"`); | ||
| } | ||
| const values = requiredWorldValue(dataLine, "list download row").split(","); |
There was a problem hiding this comment.
When a question, answer, or preceding attendee field contains a comma, quote, or newline, the CSV export correctly quotes that value but these split(",") calls treat its contents as extra columns. The story then reads the wrong fragment or column and fails despite a correct export; parse the download with a CSV parser before locating the question column.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 2e81e83 — the download is parsed with @std/csv, so a question, answer, or neighbouring field holding a comma or quote stays in its own column.
Generated by Claude Code
| expect( | ||
| whyValueCannotBeSent(browser.currentHtml, "order_intro_text", intro), | ||
| ).toBeNull(); | ||
| const { response } = await adminFormPost("/admin/site/order", { |
There was a problem hiding this comment.
If the order-intro form's action, CSRF wiring, or association with its Save button breaks, this scenario still posts a freshly constructed request directly to the hard-coded route and passes. The preceding control check proves only that the textarea appears somewhere in the page; submit the actual form extracted from the served HTML, extending the browser helper to disambiguate the two Save forms if needed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 2e81e83 — submitForm now prefers the form that renders every field being sent when one button wording serves two forms, so the intro goes through the rendered intro form (action, CSRF, and button association included) and the direct post is gone. The whole spec suite passes with the new selection rule.
Generated by Claude Code
|
|
||
| /** What a visitor on one of the front pages is shown. */ | ||
| export const visitorOnFrontPage = async (path: string): Promise<PageRead> => { | ||
| await enablePublicSite(); |
There was a problem hiding this comment.
If saving one of these pages accidentally disables the public site, every visitor assertion calls enablePublicSite() first and repairs the regression before checking the response. The scenarios already enable the site in their Given or owner-action setup, so this reader should only visit the requested path and observe the state left by the action.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 2e81e83 — the reader only visits; the site is switched on by each scenario's own setup (the Given step or the owner action), so a save that broke the public site now fails the story instead of being repaired.
Generated by Claude Code
The renewal errors, the guide's field name, and the logistics agent label now come from the catalog, ratcheting the leftover allowlist down. A booking answer goes through fillInAndSend so every field must be one the page really offers, the list download is read with a real CSV parser, the order-page intro is sent through the form that renders its box (submitForm now prefers the form carrying every field being sent when one button wording serves two forms), and the front-page reader no longer switches the public site on before looking. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The test dependency was added to deno.json without its lock entry, which left CI's evidence job seeing a dirty worktree after dependency caching. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The field-preference change left the fallback arm — no form renders a sent field, so every form stays in play — without deterministic coverage. Two direct tests now pin the preferred pick and the fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b82cfe4ac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| entryScript: "scripts/audit-equivalent-mutants.ts", | ||
| }), | ||
| await runInSnapshot({ | ||
| args: [...Deno.args, ...registry.flatMap((path) => ["--registry", path])], |
There was a problem hiding this comment.
When the task is invoked with an extra --registry custom.txt --write, parseOptions accepts the undocumented internal flag and this line forwards it to the snapshot child, so that file is audited and pruned; however, copyBack contains only the separately enumerated standard registry files. The command can therefore report success while silently discarding the custom file's pruning. Reject --registry outside isSnapshotChild(), or build the child arguments solely from the parent-enumerated manifest.
AGENTS.md reference: AGENTS.md:L980-L984
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 730c254. The parent now refuses a command-line --registry outright (--registry is internal to the audit, with usage), so the flag only ever reaches the snapshot child carrying the parent-enumerated manifest — the audited set and the copy-back set can no longer diverge.
Generated by Claude Code
The flag is the parent-to-child manifest. Accepted from the command line, the extra file would be audited in the snapshot but never copied back, so a --write run could silently discard its pruning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The gate surfaced two substring traps: the £10 example price also matches inside £10.20, and the short business-email warning also matches the settings nag. Both assertions now pin the full phrase. New tests cover the question move flash and redirect, the answer-delete activity log line, both required question fields, the answer form's label, and the title field's id. Five genuinely unobservable mutants — the overwritten sortOrder placeholder, a throw-only options label, an ?? over always-truthy arrays, and three never-read column keys — are recorded with proofs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/admin/builder.ts`:
- Around line 81-91: Move all user-facing provider labels into the English
message catalog and resolve them through t(...). Update the provider choices in
src/features/admin/builder.ts lines 81-91, the shared Deno Deploy option in
src/ui/templates/fields/admin.ts lines 112-116, and the built-site provider
labels in src/ui/templates/fields/admin.ts lines 152-158; preserve the existing
option values while replacing inline labels with catalog-backed translations.
In `@test/features/admin/attributes/options.test.ts`:
- Line 83: Make both required attribute lookups throw explicitly when
getAttributeWithOptions returns no record: update
test/features/admin/attributes/options.test.ts lines 83-83 before asserting
options, and lines 166-170 before mapping option text. Remove optional chaining
at these sites so missing attributes produce a direct failure while preserving
the existing assertions for found records.
In `@test/test-utils/test-browser.ts`:
- Around line 273-275: Update rendersEveryField in test-browser.ts to match
actual name attributes using an element/attribute boundary-aware pattern, so
data-name does not satisfy the form ranking check; preserve the existing
fieldNames.every behavior. Add a regression test covering a form with only
data-name and a competing form with the real submitted control, verifying the
correct form is selected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3e097361-b8c4-426b-9d3c-f97eead14bc8
⛔ Files ignored due to path filters (1)
deno.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
deno.jsonscripts/audit-equivalent-mutants.tsscripts/mutation/equivalent-mutants/features.txtscripts/mutation/equivalent-mutants/ui-templates.txtsrc/features/admin/builder.tssrc/locales/en/built-sites.jsonsrc/locales/en/guide-payments.jsonsrc/locales/en/listings-table.jsonsrc/locales/en/questions.jsonsrc/shared/listings-actions.tssrc/ui/templates/admin/guide/payments.tsxsrc/ui/templates/fields/admin.tssrc/ui/templates/fields/listing.tstest/features/admin/attributes/options.test.tstest/features/admin/builder/server.test.tstest/features/admin/questions/answers.test.tstest/features/admin/questions/forms.test.tstest/features/admin/questions/questions.test.tstest/features/admin/site/contact.test.tstest/features/admin/site/home.test.tstest/scripts/flag-values.test.tstest/scripts/i18n-coverage.test.tstest/specs/support/buyer-questions.tstest/specs/support/form-controls.test.tstest/specs/support/form-controls.tstest/specs/support/front-pages.tstest/specs/support/listings.tstest/test-utils/assertions.test.tstest/test-utils/assertions.tstest/test-utils/test-browser.test.tstest/test-utils/test-browser.tstest/ui/templates/admin/guide/payments.test.tsxtest/ui/templates/admin/listings/overview/detail-table.test.ts
…hains The builder and built-site provider labels now come from the message catalog, with the shared Deno Deploy option built on demand so the words resolve after the page's message group loads. Form ranking in the test browser now matches real name attributes only, so a data-name cannot claim a field, with a regression test. The attribute options tests throw when a fixture attribute is missing instead of optional chaining, and the regex escaping used in three places is one shared helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1748578952
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "Site Builder", | ||
| "Create New Site", | ||
| "Site Name", | ||
| "Site name", |
There was a problem hiding this comment.
When an operator opens the builder, this edited assertion still pins the headers Site Builder, Create New Site, and Built Sites from src/locales/en/builder.json. Change them to Site builder, Create new site, and Built sites so the touched page is consistently sentence-cased.
AGENTS.md reference: AGENTS.md:L231-L234
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79bbcfd. builder.json now says "Site builder", "Create new site", and "Built sites" (the built-sites list page title too), with the pins in builder/server.test.ts, builder.test.ts, and built-sites/server.test.ts updated to match.
Generated by Claude Code
| LogisticsAgentEditPanel({ | ||
| agent, | ||
| error: "Agent Name is required", | ||
| error: "Agent name is required", |
There was a problem hiding this comment.
When an operator uses the logistics-agent pages, this updated suite now sentence-cases Agent name but still pins Add Logistics Agent, Create Agent, and Add Agent, while the catalog retains several matching Title Case actions. Update these controls to Add logistics agent, Create agent, and Add agent so the touched logistics surface follows one convention.
AGENTS.md reference: AGENTS.md:L231-L234
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79bbcfd. logistics.json now says "Add logistics agent", "Create agent", and "Add agent", with the template and integration test pins updated.
Generated by Claude Code
| "questions.edit.new_answer_label": "New Answer", | ||
| "questions.edit.new_answer_placeholder": "e.g. Medium", | ||
| "questions.edit.add_answer": "Add Answer", | ||
| "questions.edit.add_answer": "Add answer", |
There was a problem hiding this comment.
Fresh evidence beyond the resolved Add question/Add answer thread is that this edited catalog still serves Question Text, Answer Options, New Answer, Edit Answer, Save Answer, and Delete Answer on the same question-management surface. Sentence-case these remaining labels and actions so the cleanup does not leave the page internally inconsistent.
AGENTS.md reference: AGENTS.md:L231-L234
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79bbcfd. questions.json now says "Question text", "Answer options", "New answer", "Edit answer", "Save answer", and "Delete answer", with the delete-page pin updated.
Generated by Claude Code
| "join.set_password.submit": "Set Password", | ||
| "join.set_password.password": "Password", | ||
| "join.set_password.confirm_password": "Confirm Password", | ||
| "join.set_password.confirm_password": "Confirm password", |
There was a problem hiding this comment.
When an invited user opens the join flow, this edited catalog now says Confirm password but the same pages still render Set Your Password, Set Password, Account Created, Password Set, Log In, and Invalid Invite. Change these to sentence case so the touched join surface does not mix two capitalization conventions.
AGENTS.md reference: AGENTS.md:L231-L234
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79bbcfd. The join flow now says "Set your password", "Set password", "Account created", "Password set", "Log in", and "Invalid invite", with the join template and route test pins updated.
Generated by Claude Code
| "settings.save_calendar_feeds": "Save calendar feed settings", | ||
| "admin.listings.remove_image": "Remove Image", | ||
| "fields.stripe.secret_key": "Stripe Secret Key", | ||
| "fields.stripe.secret_key": "Stripe secret key", |
There was a problem hiding this comment.
When payment settings validation fails, the labels changed here now read Stripe secret key and Square access token, but the matching errors still say Stripe Secret Key is required and Square Access Token is required and omit their final full stops. Sentence-case and punctuate those errors so the same fields are named consistently and complete error sentences follow the catalog rule.
AGENTS.md reference: AGENTS.md:L231-L237
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79bbcfd. The two errors now read "Stripe secret key is required." and "Square access token is required." — matching the field labels and ending as full sentences.
Generated by Claude Code
| "setup.complete.dashboard_link": "Go to Admin Dashboard", | ||
| "setup.complete.login_link": "Log In", | ||
| "fields.setup.username": "Admin Username *", | ||
| "fields.setup.username": "Admin username *", |
There was a problem hiding this comment.
When a new owner completes setup, this catalog now sentence-cases the admin credential labels but leaves Initial Setup, Data Controller Agreement, Your Country, Complete Setup, Setup Complete, Go to Admin Dashboard, and Log In on the same touched flow in Title Case. Update those headings, labels, buttons, and links to sentence case so the setup pages use one convention.
AGENTS.md reference: AGENTS.md:L231-L234
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79bbcfd. The setup flow now says "Initial setup", "Data controller agreement", "Your country", "Complete setup", "Setup complete", "Go to admin dashboard", and "Log in", with the setup tests and the shared e2e flows (test-utils/e2e.ts, e2e-payments) updated to match.
Generated by Claude Code
| "holidays.fields.end_date": "End Date", | ||
| "holidays.fields.end_date_hint": "Must be on or after the start date", | ||
| "fields.holiday.name": "Holiday Name", | ||
| "fields.holiday.name": "Holiday name", |
There was a problem hiding this comment.
When an operator manages holidays, the field labels changed here now read Holiday name, Start date, and End date, but the same catalog and touched tests still render Add Holiday, Create Holiday, Edit Holiday, Delete Holiday, and Upcoming Holidays. Sentence-case those remaining actions and headings so the holiday surface is consistent.
AGENTS.md reference: AGENTS.md:L231-L234
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79bbcfd. holidays.json now says "Add holiday", "Create holiday", "Edit holiday", "Delete holiday", and "Upcoming holidays", with the template, dashboard, and integration test pins updated.
Generated by Claude Code
…iday copy The remaining Title Case actions, headings, and links on the surfaces this branch touched now follow the sentence-case rule, and the two payment credential errors name their fields the way the labels do and end with full stops. Test pins and the shared e2e flows follow suit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The questions, holidays, builder, password, and logistics catalogs had a few more Title Case holdouts next to the labels the last commit fixed. They now follow the same rule, with test pins updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7a69125c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await assertUnchanged(root, entry); | ||
| const after = await Deno.readTextFile(join(workRoot, entry.file)); | ||
| if (after === entry.before) continue; | ||
| await Deno.writeTextFile(join(root, entry.file), after); |
There was a problem hiding this comment.
If this write fails after opening and truncating the live registry file—for example because the disk fills or the process is interrupted—the current file is left partial or empty, but it is not added to written until the promise succeeds, so the catch block restores only earlier files. Write to a temporary file and atomically rename it into place so a failed --write audit cannot corrupt uncommitted registry work.
AGENTS.md reference: AGENTS.md:L98-L98
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c63fa7d. Copy-back writes now go through writeWholeOrNotAtAll: the text lands in a sibling .writing temp file first and is renamed into place, so a write that dies part-way (full disk, killed process) leaves the live file holding either the old text or the new — never a truncated middle. The existing copy-back behaviour tests (preflight, rollback, leave-alone) all still pass.
Generated by Claude Code
| "fields.setup.confirm_password": "Confirm Admin Password *", | ||
| "error.agreement_required": "You must accept the Data Controller Agreement to continue", | ||
| "fields.setup.confirm_password": "Confirm admin password *", | ||
| "error.agreement_required": "You must accept the Data controller agreement to continue", |
There was a problem hiding this comment.
When setup is submitted without accepting the agreement, this edited catalog value is rendered as a complete validation-error sentence but still lacks terminal punctuation. Add a full stop so the setup error follows the repository's error-message and sentence-punctuation rules.
AGENTS.md reference: AGENTS.md:L222-L237
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c63fa7d — the error now reads "You must accept the Data controller agreement to continue." The setup test asserts via stringContaining, so it stays green with the full stop in place.
Generated by Claude Code
The logistics catalog change renamed the heading, and the tab's own template tests still pinned the old capitals — the one place the earlier sweep missed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The setup agreement error now ends as a full sentence. A copy-back write goes through a sibling temp file and a rename, so a write that dies part-way — a full disk, a killed process — leaves the live registry file holding either the old text or the new, never a truncated middle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c63fa7dd78
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const { after, before, file } of written) { | ||
| try { | ||
| if ((await Deno.readTextFile(join(root, file))) === after) { | ||
| await Deno.writeTextFile(join(root, file), before); |
There was a problem hiding this comment.
Fresh evidence beyond the resolved forward-write issue is that the rollback still writes directly to the live registry file: when copying a later shard fails after this run updated an earlier shard, an interrupted or disk-full Deno.writeTextFile here can truncate that earlier shard while trying to restore it. Reuse the atomic temporary-file-and-rename helper for the rollback so this failure path cannot corrupt uncommitted registry work.
AGENTS.md reference: AGENTS.md:L98-L98
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4fdf504. The helper moved to scripts/mutation/write-whole.ts and the rollback restore in putBackOwnWrites now goes through it too, so an interrupted restore also leaves the live shard holding either its old text or its new — never truncated.
Generated by Claude Code
| await runInSnapshot({ | ||
| args: [...Deno.args, ...registry.flatMap((path) => ["--registry", path])], | ||
| // Only a --write audit rewrites the registry, so only it keeps files. | ||
| copyBack: options.write ? registry : [], |
There was a problem hiding this comment.
When --write prunes one registry shard and then fails while writing a later shard, the child exits nonzero with a partial snapshot, but runInSnapshot still copies every configured path back before combining the copy result with the child exit code (scripts/mutation/isolation.ts:156-163). Consequently this line can publish partial or truncated registry changes to the live checkout even though the audit reports failure; only copy the manifest back after a successful child exit, or make the child's multi-shard pruning atomic.
AGENTS.md reference: AGENTS.md:L98-L98
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4fdf504, taking the second option you offered: the child's shard prune inside the snapshot now writes through the same whole-or-nothing helper, so no truncated shard can ever exist in the work copy for the copy-back to publish. What a crashed --write audit can leave is only a mix of pruned and not-yet-pruned shards — and since every registry record stands alone, that mix is a valid registry (the unpruned entries are simply still listed), which the next audit finishes. Copy-back-on-failure itself is deliberate, pinned by the "keeps the edit even when the run itself reports a failure" test in test/scripts/mutation/isolation/copy-back.test.ts, so gating on the child's exit code would change designed behaviour; removing the truncation hazard closes the actual corruption path.
Generated by Claude Code
| ): Promise<void> => { | ||
| await enablePublicSite(); | ||
| const browser = await openAdminPage(world, "/admin/site/order"); | ||
| await fillInAndSend(browser, { order_enabled: "true" }, "Save"); |
There was a problem hiding this comment.
Fresh evidence beyond the resolved listing-checkbox thread is that this new order-page story still injects a caller-chosen checkbox value: fillInAndSend verifies that order_enabled exists, but whyBoxCannotCarry compares exact values only for hidden inputs, and TestBrowser.submitForm then replaces the rendered checkbox value with "true". If the control remains visible but starts sending another value, the story enables the page and passes even though a real browser would submit that other value and toggleHandler would treat it as false; derive the value with checkboxValueOffered or require that the rendered checkbox sends exactly "true" before submission.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4fdf504. ownerTurnsOrderPageOn now calls requireCheckboxOffered(browser.currentHtml, "order_enabled", "true") before ticking, the same check the listing tabs use — if the rendered box ever stops sending exactly "true", the story fails naming the values the page actually offers instead of injecting its own.
Generated by Claude Code
The atomic write helper moves to its own module and now covers all three registry writes: the audit's shard prune inside the snapshot, the copy-back into the live checkout, and the rollback restore. A crash at any of them leaves each shard holding the old text or the new — never a truncated middle — and since each registry record stands alone, a partly-pruned set of shards is still a valid registry. The order-page story now checks the rendered box really sends "true" before ticking it, the same way the listing tabs do. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
CI's type check caught the prune write handing a possible URL to the string-only atomic writer. A shared registryFilePath helper now turns either form into a plain path, used by the prune write and the copy-back manifest, with both forms pinned by direct tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
stefan-burke
changed the title
2 participants
What this is about
Mutation testing changes the code on purpose — flipping a
truetofalse, blanking a label, removing a step — and checks that a test notices. A change no test notices is called a survivor, and every survivor is a mistake our tests would have let ship. An earlier branch surfaced a batch of survivors in the files it touched. This pull request hunts down every one of them, and while doing so, rewrites the admin tests for questions, attributes, and the public front pages as Cucumber stories that read like plain English.What changed
Every survivor is now dealt with. Most got a new or sharper test that fails the moment the code is changed. Two were hiding behind sneaky matching traps: a test that looked for the example price "£10" also matched inside "£10.20", and a test that looked for "Set a business email" also matched a different banner saying almost the same words — both now check the full sentence. A handful of changes are genuinely impossible for any test to see (for example, a placeholder number that is always overwritten before anything reads it); each of those is recorded in the known-equivalents list with a written proof of why.
The questions, attributes, and front-page tests are now stories. Scenarios like "the owner asks buyers a question" or "a visitor reads the order page" live in
specs/servicing/as Cucumber features. The story helpers only press buttons and tick boxes the page really offers — a box that stops sending the expected value fails the story by name instead of being quietly worked around.The words people see got a tidy-up. Labels, buttons, and headings on the pages this branch touched now follow the sentence-case rule ("Add holiday", not "Add Holiday"), error messages end as full sentences, and the last hard-coded labels (the site builder's provider choices, the payments guide, the logistics form) moved into the message catalog so every page says things the same way.
The test tooling got safer. The known-equivalents list now lives as small per-area files with an audit that verifies every record. Registry writes go through a whole-or-nothing write, so a crash mid-write can never leave a half-written file. The audit also refuses a hand-passed file list, so what it checks and what it saves back can never differ.
The score
fields/admin.ts,fields/listing.ts🤖 Generated with Claude Code
https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7