Harden the build-verify gate and fix the revise-rating spec contradiction

.agents/skills/scaffold-code-app/SKILL.md

@@ -71,15 +71,54 @@ sign-in is handled later by the `dataverse-scaffold-tables` skill — don't run
 ### 4. Set up the quality tooling (make the app verifiable)
 
 The Vite starter ships **no test runner** and at most a minimal linter, but every build task is gated
-by `npm run verify`. Make the project verifiable now, before any feature work:
+by `npm run verify`. Make the project verifiable now, before any feature work. Do not paraphrase this
+list or trust a gate that merely "looks right": the gate only protects the build if it runs the
+**compiler**, so set the scripts exactly as below.
 
-- Add **Vitest + React Testing Library + jsdom**, wired to `npm test`.
-- Add **ESLint + typescript-eslint + eslint-plugin-react-hooks**, wired to `npm run lint`.
-- Add an **`npm run verify`** script that runs typecheck + lint + test in one command.
+**a. Test runner.** Add **Vitest + React Testing Library + jsdom**, wired to `npm test`. Add
+`src/setupTests.ts` importing `@testing-library/jest-dom`, referenced from the Vitest config.
 
-See **AGENTS.md (repo root) → Stack notes** for the exact pins. After this, `npm run verify` runs
-clean on the empty app, so the first task gate (Task 1) has a working harness to run against rather
-than being set up live.
+**b. Linter.** Add **ESLint + typescript-eslint + eslint-plugin-react-hooks** (plus the Vite
+`react-refresh` plugin the starter expects), wired to `npm run lint`. Set the ESLint
+`languageOptions.ecmaVersion` to **2022** to match the app's `target`, not the starter's `2020`.
+
+**c. The gate must typecheck.** Set these `package.json` scripts **verbatim**. The typecheck step is
+what catches the class of error a fast model produces (reading a response by the wrong property, a
+string where a Choice integer is required, a create payload missing a required field). A gate without
+it passes green while the build is broken:
+
+```json
+"typecheck": "tsc -b",
+"lint": "eslint .",
+"test": "vitest",
+"verify": "npm run typecheck && npm run lint && npm run test -- --run"
+```
+
+`npm run verify` **must** run `tsc`. If it only runs lint and test, type errors stay hidden until
+`npm run build` at deploy. That is a configuration failure, the exact failure mode this harness exists
+to remove.
+
+**d. Keep test types out of the app build, but still typecheck the tests.** Tests must not force their
+globals into the production compile, yet they must still be type-checked by the gate. Three moves:
+
+- In `tsconfig.app.json`, **exclude** the test files and keep `types` to app types only (no
+  `vitest/globals`):
+
+  ```json
+  "exclude": ["src/**/*.test.ts", "src/**/*.test.tsx", "src/setupTests.ts"]
+  ```
+
+- Add a **`tsconfig.test.json`** that includes the test files and carries the test globals:
+  `"types": ["vite/client", "vitest/globals", "@testing-library/jest-dom"]`.
+- **Reference both** from the root `tsconfig.json` so `tsc -b` (the gate's typecheck and the build's
+  first half) checks the app and test projects in one pass.
+
+This is why the app build never needs `vitest/globals` bolted into `tsconfig.app.json`: the tests get
+their globals from their own project, and a wrong fixture shape still fails `tsc` at the gate.
+
+See **AGENTS.md (repo root) → Stack notes** for the exact pins and the test-authoring rules (typed
+fixtures, one un-mocked smoke render). After this, `npm run verify` runs clean on the empty app, so
+the first task gate (Task 1) has a working harness to run against rather than being set up live.
 
 ## After scaffolding
 

.claude/skills/scaffold-code-app/SKILL.md

@@ -71,15 +71,54 @@ sign-in is handled later by the `dataverse-scaffold-tables` skill — don't run
 ### 4. Set up the quality tooling (make the app verifiable)
 
 The Vite starter ships **no test runner** and at most a minimal linter, but every build task is gated
-by `npm run verify`. Make the project verifiable now, before any feature work:
+by `npm run verify`. Make the project verifiable now, before any feature work. Do not paraphrase this
+list or trust a gate that merely "looks right": the gate only protects the build if it runs the
+**compiler**, so set the scripts exactly as below.
 
-- Add **Vitest + React Testing Library + jsdom**, wired to `npm test`.
-- Add **ESLint + typescript-eslint + eslint-plugin-react-hooks**, wired to `npm run lint`.
-- Add an **`npm run verify`** script that runs typecheck + lint + test in one command.
+**a. Test runner.** Add **Vitest + React Testing Library + jsdom**, wired to `npm test`. Add
+`src/setupTests.ts` importing `@testing-library/jest-dom`, referenced from the Vitest config.
 
-See **AGENTS.md (repo root) → Stack notes** for the exact pins. After this, `npm run verify` runs
-clean on the empty app, so the first task gate (Task 1) has a working harness to run against rather
-than being set up live.
+**b. Linter.** Add **ESLint + typescript-eslint + eslint-plugin-react-hooks** (plus the Vite
+`react-refresh` plugin the starter expects), wired to `npm run lint`. Set the ESLint
+`languageOptions.ecmaVersion` to **2022** to match the app's `target`, not the starter's `2020`.
+
+**c. The gate must typecheck.** Set these `package.json` scripts **verbatim**. The typecheck step is
+what catches the class of error a fast model produces (reading a response by the wrong property, a
+string where a Choice integer is required, a create payload missing a required field). A gate without
+it passes green while the build is broken:
+
+```json
+"typecheck": "tsc -b",
+"lint": "eslint .",
+"test": "vitest",
+"verify": "npm run typecheck && npm run lint && npm run test -- --run"
+```
+
+`npm run verify` **must** run `tsc`. If it only runs lint and test, type errors stay hidden until
+`npm run build` at deploy. That is a configuration failure, the exact failure mode this harness exists
+to remove.
+
+**d. Keep test types out of the app build, but still typecheck the tests.** Tests must not force their
+globals into the production compile, yet they must still be type-checked by the gate. Three moves:
+
+- In `tsconfig.app.json`, **exclude** the test files and keep `types` to app types only (no
+  `vitest/globals`):
+
+  ```json
+  "exclude": ["src/**/*.test.ts", "src/**/*.test.tsx", "src/setupTests.ts"]
+  ```
+
+- Add a **`tsconfig.test.json`** that includes the test files and carries the test globals:
+  `"types": ["vite/client", "vitest/globals", "@testing-library/jest-dom"]`.
+- **Reference both** from the root `tsconfig.json` so `tsc -b` (the gate's typecheck and the build's
+  first half) checks the app and test projects in one pass.
+
+This is why the app build never needs `vitest/globals` bolted into `tsconfig.app.json`: the tests get
+their globals from their own project, and a wrong fixture shape still fails `tsc` at the gate.
+
+See **AGENTS.md (repo root) → Stack notes** for the exact pins and the test-authoring rules (typed
+fixtures, one un-mocked smoke render). After this, `npm run verify` runs clean on the empty app, so
+the first task gate (Task 1) has a working harness to run against rather than being set up live.
 
 ## After scaffolding
 

AGENTS.md

@@ -73,10 +73,14 @@ diverge from its spec.
   honour its "out of scope" line.
 - **Stop at the verify gate.** Run the task's checks, report against its "done when", and wait. Do
   not roll straight into the next task.
-- **The gate is one command.** `npm run verify` chains typecheck + lint + test so the check is
-  mechanical, not remembered. Run it at every task gate; `npm run build` runs once before deploy.
+- **The gate is one command, and it must compile.** `npm run verify` chains **typecheck + lint +
+  test** (`npm run typecheck && npm run lint && npm run test -- --run`, where `typecheck` is `tsc -b`).
+  The typecheck is not optional: drop it and a wrong response shape, a string where a Choice integer is
+  required, or a create payload missing a required field all pass the gate and only surface at
+  `npm run build` on deploy. Run `verify` at every task gate; `npm run build` runs once before deploy.
   There is no CI or git hook: the demo build is a throwaway local project (no GitHub repo on the day),
-  so `verify` at each gate is the enforcement.
+  so `verify` at each gate is the enforcement, which is exactly why it has to run the compiler and not
+  just lint and test.
 - **Per task:** typecheck + lint (near-instant). **On the data-layer task:** the audit pass (the
   checklist in tasks.md Task 1). The test suite exists from Task 1's smoke test and peaks at the
   modal milestone (Task 3).
@@ -93,9 +97,25 @@ diverge from its spec.
 - The template does **not** ship a test runner. Add **Vitest + React Testing Library + jsdom**
   explicitly, wired to `npm test`. Tests mock the generated `Eppc_*Service` modules (schema §9) and
   never touch live Dataverse.
+- **Typecheck the tests, but keep their globals out of the app build.** Test files live under `src`
+  but must be excluded from `tsconfig.app.json`; give them a `tsconfig.test.json` carrying
+  `vitest/globals`, and reference both from the root `tsconfig.json` so `tsc -b` checks them together.
+  Never add `vitest/globals` to `tsconfig.app.json` to make the build pass; that is the app build
+  compiling tests, which the exclusion fixes.
+- **Derive fixtures from the generated model types; never hand-author the shape.** Build mock sessions
+  and feedback as typed values (`const s: Eppc_sessions = { ... }`, or a small typed factory) and read
+  service results by their real property names (the SDK returns `response.data`, not `.value`). A mock
+  that invents its own shape will agree with a wrong app and pass while the build breaks; a typed
+  fixture turns the same mistake into a `tsc` error. The create payload must satisfy the generated
+  `Eppc_feedbacks` model, including any required system field it types (e.g. `statecode: 0`).
+- **Keep one un-mocked smoke render.** Alongside the mocked unit tests, the suite must include one test
+  that imports and renders `App` through the **real module graph**, mocking only at the `Eppc_*Service`
+  boundary, not your own modules. Mocking the module under test hides import-resolution failures and
+  default-vs-named import mistakes that throw at runtime (`type is invalid ... got: undefined`) while
+  the mocked suite stays green.
 - Lint is a named gate, but the template's ESLint setup is minimal or absent. Pin **ESLint +
   typescript-eslint + eslint-plugin-react-hooks** on `npm run lint`; the react-hooks rule catches a
-  class of bug a fast model will produce.
+  class of bug a fast model will produce. Set `ecmaVersion` to **2022** to match the app's `target`.
 - Load `Barlow` and `Barlow Semi Condensed` from Google Fonts (contract §1.6).
 
 ## When something is ambiguous

specs/dataverse-schema.md

@@ -137,7 +137,9 @@ columns:
 
 ## 4. Feedback — `eppc_feedback`
 
-Written at runtime, one row per rating submission. Anonymous by design.
+Written at runtime: **one row per rating**, created on first submit and **updated in place** when the
+attendee revises it (the card's Edit affordance, instantiation §3.5). Not one row per submission, a
+revise must not duplicate. Anonymous by design.
 
 ```yaml
 table:
@@ -302,14 +304,33 @@ read_feedback_for_session: # rated state / counts
     select: [eppc_rating, eppc_comments, _eppc_sessionid_value]
     filter: _eppc_sessionid_value eq <sessionGuid>
 
-create_feedback: # Rate modal submit
+create_feedback: # Rate modal submit (first rating for this session)
     service: Eppc_feedbacksService.create
     payload:
         eppc_rating: 5 # integer 1..5, NOT "5"
         eppc_comments: "Great session" # omit/empty when optional and blank
         "eppc_SessionId@odata.bind": "/eppc_sessions(<sessionGuid>)"
+        statecode: 0 # Active; the generated model types statecode as required on create
+
+update_feedback: # Rate modal submit when revising an existing rating (the Edit affordance)
+    service: Eppc_feedbacksService.update
+    id: <eppc_feedbackid returned by the create call, held in app state>
+    changedFields:
+        eppc_rating: 3 # the revised integer 1..5
+        eppc_comments: "Sharper on reflection" # omit/empty when optional and blank
 ```
 
+> **Revise updates, it does not create.** The Rate modal's Edit affordance (instantiation §3.5, §4)
+> revises a rating. Hold the `eppc_feedbackid` that `create` returns in app state and call `update` on
+> it; calling `create` a second time duplicates the row. Anonymity sets the boundary: there is no
+> attendee key, so "the existing rating" means the row this app session created, not one matched across
+> devices. Cross-device dedupe is deliberately out of scope (§4 anonymity note).
+>
+> **System fields on create.** The generated `Eppc_feedbacks` model types `statecode` (and may type
+> others) as **required** even though Dataverse defaults it (Active = `0`) on create. Satisfy the
+> generated type: send `statecode: 0`. The generated model is the source of truth for the payload
+> shape, and `tsc` at the task gate is what surfaces a missing field, not the deploy build.
+
 ---
 
 ## 9. BDD scenarios (data behaviour)
@@ -354,4 +375,13 @@ Feature: Submitting session feedback to Dataverse
     When they submit and Eppc_feedbacksService.create resolves
     Then that session's card renders its rated footer
     And the rated count increments by one
+
+  Scenario: Revising a rating updates the existing row, not creates a new one
+    Given a session rated 4 in this app session, whose feedback id is held in state
+    And the Rate modal is reopened via the card's Edit affordance
+    When the attendee changes the rating to 3 and submits
+    Then Eppc_feedbacksService.update is called with that feedback id
+    And Eppc_feedbacksService.create is not called again
+    And the session still has exactly one feedback row
+    And the rated count does not change
 ```

specs/session-feedback-instantiation.md

@@ -118,8 +118,10 @@ Per the contract content card (4.6), filled as:
   hover previews; submit requires a non-zero draft (and a comment when `requireComment`); success
   animates once then rests (contract 5.2-5.5).
 - **Persistence:** ratings persist via the app's data layer with optimistic reflect (contract 5.6).
-  In the build app this is `Eppc_feedbacksService.create` to Dataverse (see the Dataverse schema); the
-  standalone reference uses `localStorage` key `eppc26_ratings`, merging seed data under saved data.
+  In the build app a **first** rating is `Eppc_feedbacksService.create` and a **revise** (the card's
+  Edit affordance, §3.5) is `Eppc_feedbacksService.update` on the `eppc_feedbackid` the create returned
+  (Dataverse schema §8), never a second `create`, which would duplicate the row. The standalone
+  reference uses `localStorage` key `eppc26_ratings`, merging seed data under saved data.
 - **Counts:** `ratedCount` = sessions with a rating > 0; `totalCount` = all sessions.
 
 ---

specs/tasks/task-1-data-layer.md

@@ -31,7 +31,10 @@
 - One smoke test proves the data layer: mocking the generated service, it asserts the read path
   returns typed sessions ordered by `eppc_starttime`. The test runner itself was set up by
   `scaffold-code-app`; this task just adds the first test, giving the data layer its own automated
-  gate so the Task 3 milestone runs against an already-exercised harness.
+  gate so the Task 3 milestone runs against an already-exercised harness. The fixture is a typed
+  `Eppc_sessions[]` value (not a hand-shaped object) and the read is taken from the service result's
+  real `response.data` shape, so any drift from the generated model fails `tsc` rather than passing a
+  self-consistent mock (AGENTS.md Stack notes).
 
 **Done when:**
 - `tsc` is clean.

specs/tasks/task-3-rate-modal.md

@@ -27,16 +27,27 @@
 - The rate modal as specified — track-coloured top bar, header, identity row, star picker with live
   word caption, optional comment field, and submit-button states that name the blocking reason in the
   label until valid. The spec sections own the copy and the states.
-- The write that closes the loop: `Eppc_feedbacksService.create` on submit with the lookup bound via
-  `@odata.bind`, the card reflecting the result optimistically, and counts updating — this is the
-  task's reason to exist.
+- The write that closes the loop: `Eppc_feedbacksService.create` on a first submit with the lookup
+  bound via `@odata.bind`, the card reflecting the result optimistically, and counts updating — this is
+  the task's reason to exist.
+- **Revise updates, it does not create:** the card's Edit affordance reopens the modal and a second
+  submit calls `Eppc_feedbacksService.update` on the `eppc_feedbackid` the create returned (schema §8),
+  never a second `create`. One row per session, not one per submission.
 - The success state, then return to the list.
 
 **Done when (milestone — run the tests here):**
 
 - `tsc` and lint are clean, and the **test run passes** the schema §9 BDD scenarios (rating maps to
   the integer, submit blocked at draft 0, optional comment omitted when blank, required comment
-  blocks submit, one card per seeded session).
+  blocks submit, one card per seeded session, **and revising updates the existing row rather than
+  creating a second**).
+- The suite includes **one un-mocked smoke render** of `App` through the real module graph (mocking
+  only the `Eppc_*Service` boundary), so a default-vs-named import or import-resolution break is caught
+  here rather than in the browser console (AGENTS.md Stack notes).
+- The create payload satisfies the generated `Eppc_feedbacks` model: rating is the Choice **integer**,
+  the lookup is `"eppc_SessionId@odata.bind"` (never `_eppc_sessionid_value`), and required system
+  fields are present (`statecode: 0`). A quick re-run of the Task 1 audit checklist against the write
+  payload, now that it exists.
 - Submitting a rating creates a real Feedback row in Dataverse and the card flips to the rated state (manual check on the live app, not part of the mocked suite).
 - Submit disabled-state labels match the spec (`Select a rating to submit`, `Add a comment to
 submit`).