Harden the build-verify gate and fix the revise-rating spec contradiction - #7
Merged
Merged
Conversation
…tion Most demo failures only surfaced at the deploy build because the gate did not run the compiler: the scaffold skill said "add a verify script" and the fast model wrote lint+test, dropping tsc. Make the factory produce the right config by construction, and fix the spec defect behind duplicate feedback rows. - scaffold-code-app (.agents + .claude): prescribe exact package.json scripts so verify = typecheck (tsc -b) + lint + test; split test types into tsconfig.test.json so they stop polluting the app build; pin eslint to es2022 - AGENTS.md: gate must compile; fixtures typed from the generated model; one un-mocked smoke render through the real module graph - dataverse-schema.md: add update_feedback contract + statecode note; §4 one row per rating not per submission; §9 BDD scenario for revise-updates-not-creates - instantiation §4 and task-1/task-3 gates updated to match Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Hardens the repository “harness” so the per-task npm run verify gate actually catches TypeScript contract drift (and build-only failures), and resolves the spec contradiction that caused “revise rating” to create a second Dataverse Feedback row instead of updating the existing one.
Changes:
- Updates Task 1 + Task 3 specs to require typed fixtures, correct service response shape usage, an un-mocked App smoke render, and revise-via-
updatebehavior. - Updates the Dataverse schema spec to define an explicit
update_feedbackcontract and adds a BDD scenario for revise semantics. - Makes the scaffold skill + AGENTS.md prescriptive about
verifyincludingtsc -band about splitting app vs test TS configs to avoid test globals leaking into production builds.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/tasks/task-3-rate-modal.md | Expands Task 3 acceptance criteria to include revise-via-update and stronger test/gate requirements. |
| specs/tasks/task-1-data-layer.md | Tightens Task 1 testing guidance to use typed fixtures and the real service response shape. |
| specs/session-feedback-instantiation.md | Aligns instantiation behavior text with revise-via-update persistence semantics. |
| specs/dataverse-schema.md | Defines update_feedback and adds revise-related BDD coverage and guidance. |
| AGENTS.md | Makes the verify gate explicitly compile-first and documents typed fixtures + smoke render rules. |
| .claude/skills/scaffold-code-app/SKILL.md | Makes scaffold instructions prescriptive for scripts/tsconfigs so the gate compiles and tests don’t pollute app build. |
| .agents/skills/scaffold-code-app/SKILL.md | Same scaffold hardening as the .claude copy for agent parity. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+33
to
+35
| - **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. |
Comment on lines
+315
to
+320
| 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 |
Comment on lines
+379
to
+386
| 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 |
Comment on lines
+81
to
+83
| **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`. |
Comment on lines
+81
to
+83
| **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`. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A run-through of the Code App demo surfaced a cluster of failures that only appeared at the deploy build, not at the per-task gate: wrong response shape (
.valuevs.data), string-where-Choice-integer track values, a create payload missingstatecode, test globals leaking into the production build, a runtimetype is invalid ... got: undefined, and revising a rating creating a second Dataverse row.Almost all of these trace to one thing: the gate didn't run the compiler.
AGENTS.mdandtasks.mdboth claimnpm run verify"chains typecheck + lint + test", but the scaffold skill only told the agent to "add a verify script", so the fast model wrotelint && testand droppedtsc. That is a configuration failure, not a model failure (Day 1 / Day 5), which is exactly what this harness exists to prevent. The fixes go into the factory (the scaffold skill andAGENTS.md) and the spec (the asset), never into the throwawaysrc/.What changed (harness only, no app code)
Gate now compiles
scaffold-code-appskill (both.agentsand.claudecopies): step 4 is now prescriptive, with the exactpackage.jsonscripts.verify=npm run typecheck && npm run lint && npm run test -- --run,typecheck=tsc -b.AGENTS.md: the gate bullet now states the typecheck is non-optional and names the failure mode it catches.Test types stop polluting the app build
*.test.*andsetupTests.tsfromtsconfig.app.json; movevitest/globalsinto a referencedtsconfig.test.jsonsotsc -bstill typechecks the tests. Removes the need to boltvitest/globalsonto the app config.Mocks can no longer drift from the real contract
AGENTS.mdStack notes: fixtures must be typed from the generated model (so.value-vs-.data, string-vs-integer, and a missingstatecodebecometscerrors, not green-but-wrong mocks), plus one un-mocked smoke render ofAppthrough the real module graph (catches the default-vs-named import / import-resolution runtime crash the mocked suite hid).task-1andtask-3gates reference these rules.The revise-a-rating bug was a spec contradiction, so it's fixed in the spec
createfor persistence, so a revise duplicated the row.dataverse-schema.mdnow defines anupdate_feedbackcontract (Eppc_feedbacksService.updateon the idcreatereturns), §4 says one row per rating not per submission, §9 adds a BDD scenario asserting revise updates and does not create.session-feedback-instantiation.md§4 andtask-3updated to match. The anonymity boundary (no cross-device dedupe) is stated explicitly.Minor
ecmaVersionpinned to 2022 to match the app'starget(was the starter's 2020).Whitepaper alignment
App.tsx.No
src/,package.json, ortsconfig*files are committed: those are generated fresh by the scaffold skill on the day, so the fix lives in the skill's instructions, which now produce the correct config by construction.