Skip to content

C3-1b: the inventory read on the contract - #48

Merged
futurebuildai merged 4 commits into
refactor/v1from
refactor/c3-1b-inventory-read
Oct 8, 2026
Merged

futurebuildai merged 4 commits into
refactor/v1from
refactor/c3-1b-inventory-read

Conversation

@futurebuildai

Copy link
Copy Markdown
Contributor

C3-1b, the inventory read ADR 0006 7.2 states, held from C3-1 until C2-2 merged (ADR 0006 9.1). The read side only: no write path of inventory/service.go, none of the C2-2b Qty functions, no cycle 2 module.

What changes

  • GET /api/v1/inventory is the levels list on the ADR 0001 envelope, keyset paged on (created_at, id) newest first: items, next_cursor, limit, total under include=total.
  • Every quantity is a scale 4 decimal string in the product's stocking unit, with available (quantity - allocated) and uom beside it; location_id and location_name (the location's path, else the row's legacy text) replace the deprecated location float row; the product summary (id, sku, description, stock_uom) is embedded under include=product and only then (the expansion ADR 0001 section 1 names).
  • product_id and location_id are optional filters that filter (no product_id is the whole levels list, where the base 400'd); unknown parameters, repeated filters, non-UUID filter values, bad limits, malformed or foreign cursors and unknown include names are 400s naming the field in the ADR 0001 error envelope (a non-UUID product_id was a 500 on the base).
  • The branch wall PR 44 settled is kept statement for statement in the page and the new include=total count, and is pinned through serve's real wiring (TestBranchWall_InventoryListGrants reads the envelope now).
  • Migration 098 gives inventory the created_at its ordering keys on (backfilled from updated_at, NULLs filled, NOT NULL with a default) and the keyset index; the down drops both.
  • Desk and e2e readers updated: InventoryService reads items, the stock tab, yard lookup and transfer modal sum and display the decimal strings through the money helpers, the fulfilment spec helper reads available.
  • Goldens inventory and inventory_transfer re-recorded; three CONTRACT-CHANGES rows. The integration and AI load management goldens are untouched.

Tests

Failing first on the base commit b5151dc (bare array, floats, no envelope): the new wire tests and the serve wall test, shown failing in a throwaway worktree of the base. Then: wire tests (envelope, cursor walk, filters, 400s, include=product/total, empty page bytes), the migration test on a scratch database (backfill on rows that exist, down, idempotent re-up), the wall arms through serve's mounts, and Playwright on the real stack (25 passed).

No transaction is opened by this change (a read-only page and count), so the recipe's transaction proofs carry no write to prove; the wall test rides the real serve wiring as the brief requires.

Gable v1 Coder added 4 commits October 8, 2026 15:07
… facts

The levels list as the ADR 0001 envelope with the cursor walk and
include=total; each row's quantities as scale 4 decimal strings with
available (quantity - allocated) and the stocking unit's uom beside them
(ADR 0006 7.2); the product summary under include=product and only there;
product_id and location_id as filters; unknown parameters, bad values,
bad limits and malformed cursors refused naming the field. The branch
wall arm of the serve wiring test now reads the envelope, so an empty
grant list answers an empty page, never every branch's rows.

Both tests fail on the base commit b5151dc: the route answers a bare
array of float quantities with no unit, no available and no paging, and
the wall test cannot decode it as a page.
GET /api/v1/inventory is the levels list of ADR 0006 7.2: the ADR 0001
envelope, keyset paged on (created_at, id) newest first, every quantity
a scale 4 decimal string in the product's stocking unit with available
(quantity - allocated) and uom beside it, the location's name beside its
id, the product summary embedded under include=product and only then,
product_id and location_id as filters that filter, and the strict query
guard (unknown parameters, bad UUIDs, bad limits, malformed cursors and
unknown include names are 400s naming the field, where the base answered
a bare array and 500'd a bad product_id). The branch wall PR 44 settled
is kept statement for statement in the page and the count.

Migration 098 gives inventory the created_at its ordering keys on
(backfilled from updated_at, NULLs filled, NOT NULL with a default for
raw writers) and the keyset index; the down drops both. The read side
only: no write path, no Qty function, no cycle 2 module touched. The
portal's in-process read (ListByProduct) stays as the unconverted float
seam, the pattern of product's QtyFloat.
The fragment declares the envelope (InventoryLevelPage), the row
(InventoryLevel) and the product summary embedded under include=product
(InventoryProductSummary, the expansion ADR 0001 section 1 names); the
model shape test binds the two new wire structs. The goldens pin the
converted read: the empty page is [], the transfer group's list reads
the envelope with include=product and the unknown parameter's 400, and
the sales group's unfiltered page replaces the missing product_id 400
(product_id is now a filter). Three CONTRACT-CHANGES rows.
The desk's InventoryService reads items from the cursor envelope (the
levels wire of C3-1b): quantities arrive as scale 4 decimal strings with
available and uom beside them, so the stock tab and the yard lookup sum
and display them through the money helpers (the sums on the scaled
integers, one division at the end), the transfer modal lists locations by
location_name and caps the move on the parsed quantity, and the Inventory
type drops the deprecated location text. The fulfilment e2e helper reads
the page's available strings. No screen layout changes.
@futurebuildai

Copy link
Copy Markdown
Contributor Author

Review of PR 48 (C3-1b, inventory levels read), head cb654b4, base refactor/v1. Reviewer: Sonnet 5.5, round 1.

Verdict: CLEAN (no P1, no P2; three P3).

Checked

  • available and unit. available is (i.quantity - i.allocated)::text in SQL, so it is exact, computed in the NUMERIC and never in floats (levels.go ListLevelsPage). ADR 0006 7.2 defines it as quantity - allocated in the stocking unit; uom is p.uom_primary, the product's stock_uom. Both columns are NUMERIC(12,4) (migration 093), so every value parses into the scale 4 httpx.Quantity with no rounding. The wire form is the shortest exact string ("25", "124.625"), which is ADR 0001 lines 331 to 337 and the same form the product wire uses. The wall test seeds 150.125 less 25.5 and 0.0001, and the fixture asserts 124.625.
  • Migration 098. Backfill is created_at = COALESCE(updated_at, NOW()), and a NULL updated_at is filled first. Keyset order is stable (created_at is never rewritten after the migration) and total (id is the unique tiebreak). Down then up on the container's populated table ran clean: count(*) = count(created_at) and all rows have created_at = updated_at. TestMigration098_BackfillsRowsThatExist builds a scratch database to 097, seeds legacy rows and runs up, down and up again; it passes. The v_inventory_with_branch view's i.* is fixed at creation, so the added column does not disturb it.
  • Grants wall. The page and CountLevels carry the same three-arm predicate as PR 44's ListInventoryByProduct, statement for statement. Context branch gives its own rows. No context branch with a bound non-admin gives the granted branches, and empty grants match nothing. Admin without a header, unbound key or no branch context gives every branch. The legacy branchless row matches no scoped arm, so it is visible only to the last two cases, as PR 44 set it. TestBranchWall_InventoryListGrants was converted and still asserts these cases (header A, grants A, u-none gets 0 rows, admin gets 3 including legacy). The portal's in process Service.ListByProduct and ListInventoryByProduct are untouched, so the float seam is intact.
  • Cursor walk with ties. I wrote a temporary test (deleted, not committed) that set one created_at on all three rows of a product and walked at limit 1. No duplicates, none missed, id strictly descending. The row comparison (created_at, id) < ($5, $6) is correct, and the cursor time is RFC3339Nano against Postgres microseconds.
  • Filters and 400s. The strict query guard, the repeated and non-UUID filters, the bad cursor and unknown include names are all 400s that name the field. The list_unknown_param golden shows it.
  • include=product shape. It is {id, sku, description, stock_uom} and present only under the include. See P3-1.
  • Desk and e2e.
    • Read in the diffs: the stock tab (sums on scaled integers), the yard lookup (formatQuantity), the transfer modal (source options only for rows with a location_id, max quantity from parseScaled) and InventoryService, which unwraps items with limit=200.
    • Not run: screens and a local desk typecheck, because there is no node_modules in the checkout.
    • Hosted CI on cb654b4 is green: Backend, Frontend, Playwright, Local stack, App Shape, Docker, REUSE and Tauri. The fulfilment e2e now reads the envelope's available.
  • Goldens. Only inventory.json and inventory_transfer.json moved, each with its CONTRACT-CHANGES rows (three C3-1b rows: the route, the wall and count, migration 098). The request-id renumbering in the transfer golden follows from the two new steps. The characterization database is per run; four consecutive runs of the package passed, so the pinned total: 205 and the two page rows are stable.
  • Other rules.
    • New files carry the SPDX headers.
    • No transaction is opened, so no pool contention and no concurrency test is owed.
    • No dealer or person content.
    • No em or en dashes in the added lines.

Findings

  • P3-1. The embedded product is a four field subset, not the product wire. levels.go:48 defines ProductSummary{id, sku, description, stock_uom}. ADR 0006 7.1 says PIM's detail "carries the same product summary", and PIM embeds the full product.View. The names match the View fields, so the subset is consistent, but nothing ties the two. Fix: either state in 7.2 that the inventory summary is this identity subset, or add a test that pins the four JSON names to product.View's, so they cannot drift.
  • P3-2. No test for the wall on include=total, and no tied created_at in the walk test. The count's predicate is identical to the page's (checked by reading), and ties work (checked with the throwaway test above), but nothing committed fails if either regresses. TestInventoryLevelsCursorWalk inserts rows one at a time, so their created_at values differ. Fix: add include=total to the u-a and u-none cases in TestBranchWall_InventoryListGrants (expect 1 and 0), and set one created_at on the walk's rows before walking.
  • P3-3. One out of range value fails the whole page. scanLevel returns an error if ParseQuantity refuses a value. available is a difference of two NUMERIC(12,4) values and can pass QuantityMax only for a pathological row (very large negative quantity). The page then answers 500, not the other rows. Practically unreachable; a note in the code is enough.

Exit test lines run (worktree wt/rev-48 at cb654b4, container gv1-rev48-pg on 55464, URL on each command)

  • go run ./cmd/migrate applied through 098: ok.
  • make contract-go: openapi.yaml fresh (287 paths, 364 operations), 0 routes pending, apicontract ok, conformance 1110 golden steps validated, 1 known deviation.
  • go vet ./... ok; GABLE_TEST_REQUIRE_DB=1 CI=true go test -race ./...: every package ok, no failures.
  • go test -count=1 ./internal/characterization/ x4: ok each time.
  • bash scripts/check-shape.sh: check-shape: OK.
  • Manual down/098 then 098 up on the populated table: ok; 1 | 1 | 1 rows with created_at equal to updated_at.
  • Temporary tie walk test: ok (deleted afterwards, git status clean).
  • gh pr checks 48: all nine checks pass.

Not verified

Desk screens and a local typecheck of the desk (hosted CI's Frontend and Playwright jobs stand for them). Contract-ts was not run locally.

Cleanup

Worktree wt/rev-48 removed and pruned. Container removed with docker rm -f -v gv1-rev48-pg; docker ps -a --filter name=gv1-rev48 lists nothing. I started no background process. The ps -eo pid,pgid,etime,comm listing shows only older workstation processes (node, dockerd, postgres, docker-proxy, all with an elapsed time of about 4 days), none of mine.

@futurebuildai

Copy link
Copy Markdown
Contributor Author

PR 48 second review (data class), Sonnet 5.5

PR: #48 (refactor/c3-1b-inventory-read, head cb654b4) into refactor/v1.

Verdict: CLEAN (no P1, no P2; two P3 notes)

Probes (my own, on gv1-rev48b-pg port 55465, URL on each command)

  1. Migration 098 on a seeded database. Scratch database with migrations 001 to 097 applied, 3000 inventory rows (1000 with NULL updated_at, the rest sharing a fixed timestamp or spread).

    • Before: 3000|1000 (rows, NULL updated_at). After up: 3000|0|0|1002|1000 (rows, NULL updated_at, NULL created_at, distinct created_at, rows where created_at <> updated_at). Count unchanged, no NULL created_at, the 1000 NULL rows took NOW().
    • Order total: row_number() over (order by created_at desc, id desc) gave 3000 ranks; the EXPLAIN of order by created_at desc, id desc limit 10 uses an Index Only Scan on idx_inventory_created_at_id.
    • Down: count 3000, created_at and the index gone, updated_at stays NOT NULL (as the down file documents). Up again, then up a third time: idempotent (only NOTICEs), 3000|0, index present.
  2. Cursor walk. 330 rows on one product, 300 of them sharing one created_at (2026-03-03 03:03:03.123456), 30 distinct. Walks at limit 1, 7, 50, 100 returned 330, 330, 330, 330 unique rows in 330, 48, 7, 4 pages; no duplicate and no gap each time. (limit 299 answers 400: the cap is below it, so the walk was done at the cap and below.)

  3. available. Over all 330 rows, compared to Postgres quantity - allocated (trimmed canonical form): 0 mismatches before and 0 after Service.AllocateQty(12345.6789) (a C2-2b allocation). Awkward rows included: 99999999.9999 minus 0.0001, 0.0001 minus 0.0001 ("0"), 0.3 minus 0.1 ("0.2"), and an over-allocated row 1 minus 1.2345 ("-0.2345"). Quantities ride as canonical decimal strings (trailing zeros trimmed, the ADR 0001 convention the module's tests also assert).

  4. Grants wall (serve's real wiring, branch A with two rows, branch B one row, one legacy row with no location_id), page walked at limit 1 and include=total:

    caller status rows walked total
    bound u-a, granted A, no header 200 2 2
    bound u-none, no grants, no header 200 0 0
    bound u-a, header A 200 2 2
    bound u-a, header B (ungranted) 403
    bound u-none, header A 403
    admin, no header 200 4 (incl. legacy) 4
    admin, header B 200 1 1

    Page and total agree in every case; the page and count queries carry the same three arm predicate.

Exit tests run

  • CI=true DATABASE_URL=<gv1-rev48b-pg> go vet ./... clean.
  • go test -count=1 -p 1 ./internal/inventory/... ./internal/app/serve/... ./internal/apicontract/... ./internal/characterization/... ./internal/platform/... all ok.
  • go test -count=1 -p 1 ./... from core: no FAIL line.
  • Probe tests were untracked scratch files in my worktree, deleted before this report (git status clean).

Findings

  • P3 core/internal/inventory/levels.go:99 (and migration 098): created_at defaults to NOW(), the transaction start time. A row inserted by a long-running transaction that commits after a client has already walked past its created_at is not returned to that in-progress walk. Inherent to keyset on created_at; the same holds for every list on this envelope. Fix, if wanted: note it in ADR 0001, or default to clock_timestamp().
  • P3 web/apps/desk/src/services/InventoryService.ts:52: getInventoryByProduct reads only the first page (limit=200) and drops next_cursor; a product stocked in more than 200 rows would be silently truncated. Unlikely, but the comment says "fits many times over". Fix: loop on next_cursor or accept and keep the comment.

Checked and fine

SPDX headers on migration, down, levels.go and the new tests; no secret, dealer or person content; no transaction opened (reads go through GetExecutor(ctx)); PR 44's grants wall kept statement for statement and extended to the count; tests in the PR (wire_test, migration098_test, wall test) pass; CONTRACT-CHANGES rows present; no em or en dashes seen in the new docs.

Not checked

Web (vitest, tsc, e2e/Playwright) was not run; only the Go side and the desk diff read. Hosted CI status not read.

Cleanup

Worktree wt/rev-48b removed (ls wt | grep rev-48b empty), local branch pr48-rev deleted, container removed with docker rm -f -v gv1-rev48b-pg (docker ps -a --filter name=gv1-rev48b-pg empty). My background test run exited; the go processes still listed by ps -eo pid,pgid,etime,comm belong to other lanes (none started by me).

@futurebuildai
futurebuildai merged commit 1bbc3e5 into refactor/v1 Oct 8, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant