[M3] POST /slabs/{id}/review accepts owner overrides but never persists them (REVIEW-25, METADATA-50) #80

New Issue

2026-09-20T18:16:47Z

grokbot_ty commented

2026-09-20 18:16:47 +00:00

Summary

REVIEW-25 says the owner may override the price and the override sticks; METADATA-50 says typed values always win over inferred values. The review endpoint reads those edits, merges them into an in-memory dict, and returns them — but never writes to the database. The override survives exactly one response.

Found in a read-only review of main @ b99cbdf (2026-09-20). 63 backend tests pass and both docs gates pass at that commit; these are gaps the green suite does not cover.

Observed evidence

backend/review.py:142-147 merges the editable keys into the local dict; backend/review.py:159-174 derives title/short_description/price from the merged dict. No UPDATE/INSERT follows — the only writes in the file are none; the function ends by returning a dict (:175-183).

Reproduction against the real app (temp SQLite, no network):

POST /api/v1/slabs          {... price: 100.0 ...}        -> 201
POST /api/v1/slabs/s1/review {"price": 999.0}             -> 200, body price 999.0
GET  /api/v1/slabs/s1                                       -> price 100.0   (override gone)
POST /api/v1/slabs/s1/review {}                           -> 200, body price 100.0

The same applies to title, short_description, description, sku, price_source, content_source.

backend/tests/test_review.py:78-87 asserts only the response body (body["price_suggestion"] == 184.0, body["price"] == 184.0), which is why the suite stays green over a broken requirement.

Acceptance

Decide and record the contract: either POST /review persists the merged edits (and bumps server_rev), or it is a pure compute endpoint and the override path is PUT /slabs/{id} — in which case the request schema should stop advertising those keys.
A test asserts persistence: POST review with an override, then GET the slab (or call review again with no payload) and see the override survive.
price_source lands on the value the schema allows (override), not a free string — the column has a CHECK constraint and an out-of-domain value currently raises a raw sqlite3.IntegrityError.

## Summary `REVIEW-25` says the owner may override the price and **the override sticks**; `METADATA-50` says typed values always win over inferred values. The review endpoint reads those edits, merges them into an in-memory dict, and returns them — but never writes to the database. The override survives exactly one response. Found in a read-only review of `main` @ `b99cbdf` (2026-09-20). 63 backend tests pass and both docs gates pass at that commit; these are gaps the green suite does not cover. ## Observed evidence `backend/review.py:142-147` merges the editable keys into the local dict; `backend/review.py:159-174` derives title/short_description/price from the merged dict. No `UPDATE`/`INSERT` follows — the only writes in the file are none; the function ends by returning a dict (`:175-183`). Reproduction against the real app (temp SQLite, no network): ``` POST /api/v1/slabs {... price: 100.0 ...} -> 201 POST /api/v1/slabs/s1/review {"price": 999.0} -> 200, body price 999.0 GET /api/v1/slabs/s1 -> price 100.0 (override gone) POST /api/v1/slabs/s1/review {} -> 200, body price 100.0 ``` The same applies to `title`, `short_description`, `description`, `sku`, `price_source`, `content_source`. `backend/tests/test_review.py:78-87` asserts only the response body (`body["price_suggestion"] == 184.0`, `body["price"] == 184.0`), which is why the suite stays green over a broken requirement. ## Acceptance - Decide and record the contract: either `POST /review` persists the merged edits (and bumps `server_rev`), or it is a pure compute endpoint and the override path is `PUT /slabs/{id}` — in which case the request schema should stop advertising those keys. - A test asserts persistence: POST review with an override, then GET the slab (or call review again with no payload) and see the override survive. - `price_source` lands on the value the schema allows (`override`), not a free string — the column has a CHECK constraint and an out-of-domain value currently raises a raw `sqlite3.IntegrityError`. ## Related - `#77` (Review/Publish UI rebuild — the override control belongs in that form) - Requirements: REVIEW-5, REVIEW-25, METADATA-50, QUALITY-90

grokbot_ty added the blocking label 2026-09-20 18:16:47 +00:00

grokbot_ty referenced this issue from a commit

2026-09-21 08:32:29 +00:00

fix(backend): #84 real woo_reachable, #83 reject unknown fields, #82 live taxonomy in stale_field, #80 persist review overrides

grokbot_ty closed this issue

2026-09-21 17:23:40 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Ty_Tech/SlabUploader#80