diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7ca686d..6392a58 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,7 +34,7 @@ Already cloned without it? `git lfs pull` fixes it in place. The `dev` dataset i npm run check ``` -That checks, each with a committed negative test, that `SPEC.md` agrees with the requirement registry, that every MUST is covered, that the promotion index returns a superset of the eligible set, that the dataset matches its manifest, that the tree typechecks, and that a simulated deploy lands nothing dev-only on a node. +That checks, each with a committed negative test, that `SPEC.md` agrees with the requirement registry, that every reference to a section of `SPEC.md` resolves to a heading that exists, that every MUST is covered, that the promotion index returns a superset of the eligible set, that the dataset matches its manifest, that the tree typechecks, and that a simulated deploy lands nothing dev-only on a node. Then the conformance suite, which is the executable form of the specification: diff --git a/SPEC.md b/SPEC.md index 78f69ae..8d67123 100644 --- a/SPEC.md +++ b/SPEC.md @@ -6,7 +6,7 @@ Scope, methodology, and the rules governing how results may be described are def --- -## 1. What this document is +## What this document is The reference specification for the application under benchmark. It exists to be implemented more than once — on Harper, and on assembled stacks — so the implementations can be compared. @@ -26,7 +26,7 @@ P0 is two endpoints, one background writer, eight tables, and one dataset. Delib Storefront UI, auth, product listing, search, checkout commit, images, and realtime are **future work**. The reasoning behind each, and the decisions already taken on them, are preserved in [`docs/future-work.md`](docs/future-work.md) so they are not rediscovered from scratch. -## 2. How to read this document +## How to read this document Every requirement has a permanent id, `AREA-NNN`. Withdrawn requirements are struck through and their ids retired, never reused — conformance reports reference them forever. @@ -38,7 +38,7 @@ Areas: `DATA` dataset and schema · `QUOTE` cart quote · `PDP` product aggregat --- -## 3. Data model +## Data model Eight tables. Field names are normative for the API; storage representation is not specified. @@ -63,14 +63,21 @@ Reaching eight tables required three judgment calls. Each is recorded because ea `DATA-001` **MUST** — An implementation uses exactly these eight logical entities **as its source of truth**. A stack MAY represent them differently where its idiom demands (a normalized schema may split embedded line items into their own relation), and MUST then document the mapping. What it MUST NOT do is pre-join or denormalize its *source of truth* into a shape that removes a read the specification requires — that is the measurement, not an optimization. -**Derived caches are permitted and expected**, and are not a violation of the above: a cache holds a copy, not the truth. The distinction is testable — deleting every cache must change no response, only its latency. A separated stack caching the product aggregate in Redis and a collapsed stack caching it in-process are doing the same thing; what differs is the cost of keeping it coherent, which is what §6 exists to measure. +**Derived caches are permitted and expected**, and are not a violation of the above: a cache holds a copy, not the truth. The distinction is testable — deleting every cache must change no response, only its latency. A separated stack caching the product aggregate in Redis and a collapsed stack caching it in-process are doing the same thing; what differs is the cost of keeping it coherent, which is what [Background writes](#background-writes) exists to measure. ### Caching - `CACHE-001` **MUST** — Caches are **derived**. Dropping every cache changes no response body, only latency. No cached value is authoritative, and nothing is served from a cache that could not be recomputed from the eight entities. -- `CACHE-002` **MUST** — Every cached value is bounded by `FRESH_MS` (§6) — by explicit invalidation on write, by expiry, or both. An implementation states which mechanism it relies on. +- `CACHE-002` **MUST** — Every cached value is bounded by `FRESH_MS` (Background writes) — by explicit invalidation on write, by expiry, or both. An implementation states which mechanism it relies on. - `CACHE-003` **MUST** — A cache key includes every dimension the cached value varies by. Serving a value keyed on fewer dimensions than it varies by is a correctness failure, not a cache tuning choice. +### Rate table invariants + +The `rate` table is looked up, never searched, so its contents carry a contract. These hold in the dataset and an implementation may rely on them rather than defending against them: + +- **Shipping bands within a region are contiguous, non-overlapping, and inclusive at both ends.** `weightMin` and `weightMax` are both inclusive, and the bands of a region cover every non-negative weight — the topmost band is open-ended in effect. There is therefore exactly one matching band for any cart, and **no fallback behaviour is specified because no cart can reach one.** An implementation that finds zero or several matching bands has loaded a dataset that violates this contract and should fail loudly rather than guess. +- **Exactly one tax rate per jurisdiction.** + ### Invariants - `DATA-002` **MUST** — Money is integer minor units (cents) end to end. No implementation may introduce a binary floating-point representation of money. @@ -80,7 +87,7 @@ Reaching eight tables required three judgment calls. Each is recorded because ea --- -## 4. `POST /cart/:id/quote` +## `POST /cart/:id/quote` The write-shaped read path, and the primary endpoint under test. Prices a cart. @@ -94,16 +101,33 @@ The write-shaped read path, and the primary endpoint under test. Prices a cart. - `QUOTE-001` **MUST** — Every response is unique to its cart. No implementation may serve a stored response for the endpoint. Entity caches are expected and do their normal job; **response-level caching is a conformance failure**, not an optimisation. - `QUOTE-002` **MUST** — Each line resolves its product and variant. A line naming an unknown sku fails the quote with `400`; it is not silently dropped. -- `QUOTE-003` **MUST** — Availability per line is resolved against inventory across fulfillment locations, honouring location priority. -- `QUOTE-004` **MUST** — The customer's tier is **applied** to pricing, and their loyalty balance is **read and carried** in the response. +- `QUOTE-003` **MUST** — Availability per line is resolved against inventory across fulfillment locations in ascending `location.priority`, drawing from each in turn until the line's quantity is satisfied or the region's locations are exhausted. + + **A shortfall does not change what is priced.** The line is priced at its full requested quantity and the unmet amount is reported as `shortfall`. Pricing only the available quantity would make a cart's total a function of live inventory, which the background writer (Background writes) is continuously changing — totals would drift under load for reasons unrelated to the architecture, and `QUOTE-008` determinism would hold only between writes. A quote prices what was asked for and states what cannot be filled. +- `QUOTE-004` **MUST** — The customer's tier is **applied** to pricing, by the normative table in *Resolved unit price* below, and their loyalty balance is **read and carried** in the response. Carried, not applied, deliberately. Redeeming a balance is arithmetic on a field the quote already fetches in wave 2: no extra read, no extra wave, no cache pressure. It would impose a normative redemption rule — conversion, cap, position in the promotion order, treatment of the tax base — that every implementation must reproduce exactly, in exchange for distinguishing no architecture. Redemption becomes interesting at **checkout**, where it decrements a balance under concurrency: a contended per-customer write. It is recorded as future work there rather than as busywork here. - `QUOTE-005` **MUST** — Promotions are resolved by tier, SKU, and category — **including promotions unrestricted on any of those dimensions** — and evaluated in application code: **stacking, exclusivity, threshold, and BOGO** rules. Evaluation order is specified (below) so every implementation produces identical totals. -- `QUOTE-006` **MUST** — Shipping is resolved by region and total cart weight. +- `QUOTE-006` **MUST** — Shipping is resolved by the customer's region and the cart's total weight, where total weight is `Σ (variant.weight × quantity)` over the merged lines, against the band contract in Data model. Shipping is **not** discounted and **not** taxed. - `QUOTE-007` **MUST** — Tax is resolved by the customer's jurisdiction and applied to the post-discount subtotal. - `QUOTE-008` **MUST** — The quote is **deterministic**: the same cart against the same dataset state produces a byte-identical response. This is what makes the endpoint verifiable at all, and it is the ground-truth correctness guard the measurement rules require. - `QUOTE-009` **MUST** — An unknown cart id returns `404`. -- `QUOTE-010` **MUST** — The response itemizes per line: resolved unit price, quantity, applied promotions, and line total; and at cart level: subtotal, discount total, shipping, tax, and grand total. +- `QUOTE-010` **MUST** — The response itemizes per line: resolved unit price, quantity, applied promotions, line total, and `shortfall` (zero when the line is fully available); and at cart level: subtotal, discount total, shipping, tax, and grand total. `grandTotal` equals `subtotal - discountTotal + shipping + tax`. + +### Resolved unit price + +Before any promotion is considered, each line's unit price is `variant.basePrice` adjusted for the customer's tier. The adjustment is **normative** — without it two implementations disagree on every line of every quote, before pricing logic has run at all. + +| Tier | Basis points | Effect | +|---|---|---| +| `standard` | 10000 | base price unchanged | +| `silver` | 9500 | 5% off | +| `gold` | 9000 | 10% off | +| `platinum` | 8500 | 15% off | + +`resolvedUnitPrice = round_half_up(basePrice × tierBasisPoints / 10000)`, in integer minor units. A tier absent from this table resolves as `standard`. + +The product aggregate resolves its price the same way, so a given variant at a given tier prices identically on both endpoints. ### Promotion evaluation order @@ -111,22 +135,27 @@ Normative, because promotion stacking is order-dependent and an unspecified orde > **Each cart-wide promotion (`exclusive`, `threshold`) and each `bogo` promotion applies at most once per cart. Each `stackable` promotion applies at most once per eligible line.** This is the rule that decides most of the rest. Left implicit, a single cart-wide promotion eligible for several lines applies once per line and compounds — a large enough one prices the cart to zero and returns a well-formed, deterministic, entirely wrong 200. -> **All discounts draw on one budget: the line's remaining amount.** Cart-wide discounts are allocated across the lines they are eligible for, in proportion to what each still has left, by largest remainder with ties on ascending SKU. Separate cart-wide and per-line accumulators let both discount the same money, which the 60% cap then concealed. +> **All discounts draw on one budget: the line's remaining amount.** Cart-wide discounts are allocated across the lines they are eligible for, in proportion to what each still has left, by largest remainder with ties on ascending SKU. Holding a separate cart-level accumulator alongside the per-line ones lets both discount the same money; the cap bounds the result either way, so the double-spend is invisible in the total and shows up only as per-line figures that do not sum to it. -> **Cart lines are merged by SKU before evaluation.** Nothing requires a cart's lines to be distinct, and per-SKU state with repeated SKUs produced a negative discount — a quote charging above its own subtotal — for a cart with no promotions. +> **Cart lines are merged by SKU before evaluation.** Nothing requires a cart's lines to be distinct, and evaluation state is keyed by SKU. Two entries of one SKU are one line of a larger quantity — which is what a shopper expects, and what keeps per-SKU state consistent with a subtotal summed per line. -Promotions are either **cart-wide** (`exclusive`, `threshold`) or **per-line** (`bogo`, `stackable`). The two draw on separate budgets: cart-wide discounts accumulate against the subtotal, per-line discounts draw down that line's remaining amount. Neither may take a line or the cart below zero. +Promotions are either **cart-wide** (`exclusive`, `threshold`) or **per-line** (`bogo`, `stackable`). Both kinds draw down the same per-line remaining amounts, by the allocation rule above. No discount may take a line, or the cart, below zero. 1. Candidate promotions are collected by tier, SKU, and category. **A promotion unrestricted on a dimension (an empty array) is eligible on that dimension**, so a promotion unrestricted on every dimension is eligible for every line — collection must return it. 2. **Exclusive** — cart-wide, evaluated against the subtotal. The highest-value one wins; if one applies, **no other promotion applies at all**. Attributed to every line it was eligible for. 3. Otherwise **threshold** — cart-wide, each eligible promotion applied **once**, against the **pre-discount** subtotal, if the subtotal meets its threshold. Attributed to every line it was eligible for. 4. Then **BOGO** — each eligible promotion applied **once**, to the single lowest-priced qualifying unit among the lines it is eligible for, where a qualifying line has quantity ≥ 2 and a non-zero remaining amount. Ties break on ascending SKU, so the result does not depend on cart line order. + + **The discount is the promotion's own amount applied to that one unit**, not to the line — `amountBasisPoints` of the unit price, or `amountMinor`, by the same rule as every other kind. A `bogo` promotion at 10000 basis points makes that unit free, which is what the name describes and what the dataset's `bogo` rows carry; the magnitude is read from the promotion rather than assumed so that a corpus carrying anything else prices correctly. 5. Then **stackable** — each eligible promotion applied **once per eligible line**, against that line's **remaining** amount, so successive percentage discounts compound rather than all computing against the gross line total. -6. Ties at any step break on ascending promotion id. -7. **At most three stackable promotions apply to any one line**, in ascending promotion id. -8. **`discountTotal` never exceeds 60% of the subtotal.** The cap is applied last, so it bounds every path including a single large exclusive. +6. **A promotion carrying a non-zero `thresholdMinor` requires that minimum pre-discount subtotal, whatever its kind.** It is not exclusive to the `threshold` kind: a flat `amountMinor` on a stackable needs a floor, or it discounts more than the cart it lands on. +7. Ties at any step break on ascending promotion id. +8. **At most three stackable promotions apply to any one line**, in ascending promotion id. +9. **`discountTotal` never exceeds 60% of the subtotal**, floored. The cap is a **running** bound, not a final truncation: each discount is applied against the headroom remaining at the moment it is applied, and one that would cross the cap is applied **partially**, up to that headroom. A promotion reduced to nothing by an exhausted cap is not attributed to any line. + + Truncating at the end instead would leave `appliedPromotionIds` citing promotions whose discount was scaled away afterwards, which `QUOTE-012` forbids, and would make per-line totals disagree with the cart total they sum to. -Steps 7 and 8 are normative bounds, not tuning: real stores limit stacking, and without a bound a promotion corpus with enough unrestricted stackables compounds a cart to zero. +Steps 8 and 9 are normative bounds, not tuning: real stores limit stacking, and without a bound a promotion corpus with enough unrestricted stackables compounds a cart to zero. Rounding: each discount rounds half-up to the minor unit at the point it is applied, not at the end. @@ -135,26 +164,26 @@ Rounding: each discount rounds half-up to the minor unit at the point it is appl --- -## 5. `GET /product/:id?tier=®ion=` +## `GET /product/:id?tier=®ion=` The read-heavy leg. Mostly cacheable, but varies by `tier` and `region`. - `PDP-001` **MUST** — The response aggregates: the product, its variants, inventory for those variants, resolved price, review rollup, and related items. - `PDP-002` **MUST** — Resolved price varies by `tier`; availability varies by `region`. Two requests differing only in `tier` or only in `region` MUST be able to produce different responses, and an implementation that caches MUST include both in its cache key. Serving one tier's price to another is a conformance failure. -- `PDP-003` **MUST** — Inventory and price reflect background writes (§6) within `FRESH_MS`. +- `PDP-003` **MUST** — Inventory and price reflect background writes (Background writes) within `FRESH_MS`. - `PDP-004` **MUST** — An unknown product id returns `404`. - `PDP-005` **SHOULD** — Related items are returned with enough detail to render without a further request. --- -## 6. Background writes +## Background writes A steady, low-rate stream of inventory and price updates against the same records the read path touches. **Not an endpoint under test.** Its own latency is not a headline metric. It exists so that caches have to stay coherent with their source of truth — a read-only workload lets a separated stack's cache fill once and never invalidate, which is not a cache any real store operates. - `WRITE-001` **MUST** — The writer updates `inventory` quantities and `variant` prices against records within the read path's working set, at a configured steady rate, **through the application's write surface**. A write made directly to the datastore updates the source of truth while invalidating nothing, so the cache converges only on expiry and the coherence cost this section exists to measure is never paid. A separated stack's write path has to evict its cache key for the same reason; this is the same work on the other architecture. -- `WRITE-002` **MUST** — A committed write is observable on the product aggregate (§5) within `FRESH_MS`, and in quote pricing (§4) within `FRESH_MS`. +- `WRITE-002` **MUST** — A committed write is observable on the product aggregate (GET /product/:id) within `FRESH_MS`, and in quote pricing (POST /cart/:id/quote) within `FRESH_MS`. - `WRITE-003` **MUST** — No implementation may satisfy `WRITE-002` by disabling caching. Measured cache-hit rates are recorded with every run precisely so that this is visible. - `WRITE-004` **MUST** — The write stream is identical in rate and key distribution for every target. It is not a per-target tuning budget. @@ -168,7 +197,7 @@ Setting it too tight makes the benchmark a cache-invalidation test; too loose an --- -## 7. Observability +## Observability - `OBS-001` **MUST** — Both endpoints emit `Server-Timing` decomposing server-side time into at least data access, application compute, and total. - `OBS-002` **MUST** — Responses indicate cache status, and each implementation documents its caching mechanism. The measurement rules require measured cache-hit rates to be recorded with every run; this is how. @@ -176,7 +205,7 @@ Setting it too tight makes the benchmark a cache-invalidation test; too loose an --- -## 8. Conformance +## Conformance An implementation publishes a conformance report: every requirement id, and pass / fail / deviation-with-reason. @@ -196,4 +225,4 @@ P0 does not complete until this document passes: self-review → cross-model rev | Cross-model review | ☑ 2026-10-01 — two rounds, Claude + codex (agy timed out both rounds) | | Human review | ☐ | -**Known open:** `FRESH_MS` (§6), the eight-table foldings (§3), and the promotion evaluation order (§4) — the last because it is invented here rather than derived from a real system, and it determines whether two correct implementations agree on a total. +**Known open:** `FRESH_MS` (Background writes), the eight-table foldings (Data model), and the promotion evaluation order (POST /cart/:id/quote) — the last because it is invented here rather than derived from a real system, and it determines whether two correct implementations agree on a total. diff --git a/bench/README.md b/bench/README.md index b0f830b..248c552 100644 --- a/bench/README.md +++ b/bench/README.md @@ -55,7 +55,7 @@ Run `bench/run.mjs` directly and you get none of the orchestrator's guarantees: ## Why the access distribution is load-bearing -Keys are drawn with skew because uniform draws make the cache unmeasurable. A uniform draw over the catalog crossed with tier and region spans a key space a short run almost never revisits, and the measured hit rate collapses to near zero. A cache nobody asks for twice cannot be measured, and neither can the cost of invalidating it — which silently takes `WRITE-003` and most of §6 out of reach without any test failing. +Keys are drawn with skew because uniform draws make the cache unmeasurable. A uniform draw over the catalog crossed with tier and region spans a key space a short run almost never revisits, and the measured hit rate collapses to near zero. A cache nobody asks for twice cannot be measured, and neither can the cost of invalidating it — which silently takes `WRITE-003` and most of the write-coherence requirements out of reach without any test failing. The skew is defined in `@ecommerce-store/spec` and shared with the dataset generator, so the dataset's hot products and the workload's hot products are the same products. A hot subset is what makes cache-hit rate mean anything. diff --git a/bench/run.mjs b/bench/run.mjs index de2c243..62c43af 100644 --- a/bench/run.mjs +++ b/bench/run.mjs @@ -61,7 +61,7 @@ const OPS_URL = argOf('ops', 'http://localhost:9925'); * * `harper dev` disables auth; `harper run` — the only supported benchmark configuration — * does not. Without these every write 401s, `fetch` resolves normally, nothing checks the - * status, and the run record still reports `writesIssued`. The entire SPEC.md §6 + * status, and the run record still reports `writesIssued`. The entire SPEC.md#background-writes * cache-coherence workload silently does not happen. */ const OPS_AUTH = process.env.HDB_ADMIN_USERNAME @@ -308,7 +308,7 @@ async function step({ rate, seconds, nextRequest, nextWrite, writeRate, label }) writesIssued, writeFailures, // Cache entries invalidated by this step's writes. The invalidation fan-out is the cost - // SPEC.md §6 exists to measure, so it is recorded rather than inferred. + // SPEC.md#background-writes exists to measure, so it is recorded rather than inferred. invalidations, // Requests still outstanding when the drain deadline expired. They are absent from the // latency distribution, so a non-zero value means the tail is censored. diff --git a/bench/workload.mjs b/bench/workload.mjs index f85702e..d2327e6 100644 --- a/bench/workload.mjs +++ b/bench/workload.mjs @@ -1,5 +1,5 @@ /** - * The request mix — SPEC.md §4, §5, §6. + * The request mix — SPEC.md#post-cartidquote, "GET /product/:id", "Background writes". * * Deliberately shares nothing with the application. The harness must be able to drive any * implementation of the specification, so it speaks HTTP and canonical paths only. @@ -83,7 +83,7 @@ export function makeRequestFactory({ baseUrl, cartIds, productIds, quoteShare = } /** - * The background write stream — SPEC.md §6. + * The background write stream — SPEC.md#background-writes. * * Not an endpoint under test. It exists so caches have to stay coherent with their source * of truth: a read-only workload lets a cache fill once and never invalidate, which is not @@ -102,7 +102,7 @@ export function makeWriterFactory({ baseUrl, skus, inventoryIds, seed = 7 }) { * Writes go through the APPLICATION, not the operations API. * * A direct table write updates the source of truth and invalidates nothing, so the cache - * would only converge on expiry and §6's cache-coherence cost — the thing it exists to + * would only converge on expiry and SPEC.md#background-writes's cache-coherence cost — the thing it exists to * measure — would never be paid. A separated stack's write path has to evict its Redis key * for the same reason; this is the same work on the other architecture. */ diff --git a/containers/compose.yaml b/containers/compose.yaml index deb2d9d..83d35b1 100644 --- a/containers/compose.yaml +++ b/containers/compose.yaml @@ -75,7 +75,7 @@ services: TARGET_URL: http://harper:9926 OPS_URL: http://harper:9925 # The background write stream authenticates against the operations API. Without these - # every write 401s, fetch resolves normally, and the SPEC.md §6 coherence workload + # every write 401s, fetch resolves normally, and the SPEC.md the "Background writes" coherence workload # silently does not happen while the run record still reports writesIssued. HDB_ADMIN_USERNAME: ${HDB_ADMIN_USERNAME:-benchadmin} HDB_ADMIN_PASSWORD: ${HDB_ADMIN_PASSWORD:?set in containers/.env} diff --git a/docs/data-model.md b/docs/data-model.md index d501462..eac4333 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -1,6 +1,6 @@ # Data model -Status: **P0 design.** Implements SPEC.md §3. +Status: **P0 design.** Implements [Data model](../SPEC.md#data-model). SPEC.md defines the eight tables neutrally. This document records the Harper-side design and the reasoning behind the choices that are not forced. @@ -108,8 +108,7 @@ type Rate @table @export @sealed { } ``` -And the one table the specification does not describe, because it is derived rather than a -source of truth: +And the one table the specification does not describe, because it is derived rather than a source of truth: ```graphql # A cache, not an entity. CACHE-001: dropping it changes no response body, only latency. @@ -134,5 +133,5 @@ type ProductView @table(expiration: 120) { ## Open -- **The promotion evaluation order in SPEC.md §4 is invented**, not derived from a real pricing engine. It is normative because the alternative is two correct implementations disagreeing on a total — but it should be sanity-checked against someone who has built one. +- **The promotion evaluation order in [POST /cart/:id/quote](../SPEC.md#post-cartidquote) is invented**, not derived from a real pricing engine. It is normative because the alternative is two correct implementations disagreeing on a total — but it should be sanity-checked against someone who has built one. - **How selective the promotion index can get.** It narrows to 7.2% of the table on `dev`. Tier is the floor: four values means a tier probe can never return less than roughly a quarter plus the storewide share. A composite `(tierKey, categoryKey)` index, or folding tier into the category key, would go further — worth measuring at `bench` scale before building. diff --git a/docs/plan.md b/docs/plan.md index fd50b73..aa1648b 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -22,7 +22,7 @@ The benchmarks repo's methodology is binding here. Results are never published f | Piece | State | |---|---| -| `SPEC.md` | Written; two rounds of cross-model review applied. **Review gate not passed** — see SPEC.md §8, which is the only place gate status is tracked | +| `SPEC.md` | Written; two rounds of cross-model review applied. **Review gate not passed** — see [Conformance](../SPEC.md#conformance), which is the only place gate status is tracked | | Schema | Built — the eight specified tables, plus `ProductView`, a derived cache | | Caching | `ProductView`, a `sourcedFrom` cache keyed by product × tier × region, invalidated write-through with an expiry backstop | | Write surface | `POST /admin/variant/:sku`, `POST /admin/inventory/:id` — write and invalidate | @@ -38,8 +38,10 @@ The benchmarks repo's methodology is binding here. Results are never published f ## Next 1. **A `bench`-scale run that completes.** No run has yet finished the full ladder at `bench` scale, so the observations below are partial and nothing downstream of them is settled. -2. **A ground-truth correctness guard in the harness.** The measurement rules require it and the harness does not have it. This is the largest outstanding hole in the measurement story — see `bench/README.md`. -3. **Confirm `FRESH_MS`.** SPEC.md §6 now sets it provisionally; the figure wants a real run behind it, and `PDP-003` / `WRITE-002` depend on it. +2. **A correctness suite with expected values.** The measurement rules require a ground-truth guard and the harness does not have one — see `bench/README.md`. The shape: a suite separate from conformance, keyed to the loaded dataset's manifest checksum, asserting *values* rather than properties. Conformance asserts a quote is well-formed and self-consistent; two implementations can both pass it and disagree on every total, which is what this closes. + + The expected values are **data, not a shared function**. A calculator in this repo emits them; no other implementation imports anything. A stack that computes pricing inside one SQL query compares against the same table as Harper does — the answer is stack-neutral, the procedure deliberately is not. +3. **Confirm `FRESH_MS`.** [Background writes](../SPEC.md#background-writes) now sets it provisionally; the figure wants a real run behind it, and `PDP-003` / `WRITE-002` depend on it. 4. **An in-application background writer.** The harness drives writes through the operations API. That is fine for coherence pressure but is not the same as the application doing it. ## Blocked on someone else @@ -47,16 +49,13 @@ The benchmarks repo's methodology is binding here. Results are never published f | | Needed from | |---|---| | **A Linux host.** On macOS, Podman runs in a VM: CPU/memory limits bind against the VM's share, not the host's, and the VM boundary sits in the network path. Clock pinning is impossible, so no cycle-normalized efficiency figure exists. | Maintainers | -| **Review gate** — tracked in SPEC.md §8. Human review outstanding. | Maintainers | +| **Review gate** — tracked in [Conformance](../SPEC.md#conformance). Human review outstanding. | Maintainers | | **The promotion evaluation order is invented.** Normative because two correct implementations otherwise disagree on a total. Nobody who has built a pricing engine has read it. | A reviewer with pricing experience | | **OAuth authorization-server question** — drafted at [`questions/oauth-authorization-server-scope.md`](questions/oauth-authorization-server-scope.md), unsent. Not blocking; auth is future work. | `@harperfast/oauth` maintainers | ## First bench-scale observations (2026-10-05) — NOT results -A `bench`-scale run reached three ladder steps before the load generator died of heap -exhaustion. The numbers below are recorded because they are the first evidence the dataset -is doing its job, and discarded as measurements because the run did not complete, the -generator was in trouble throughout, and the clock was not pinned. +A `bench`-scale run reached three ladder steps before the load generator died of heap exhaustion. The numbers below are recorded because they are the first evidence the dataset is doing its job, and discarded as measurements because the run did not complete, the generator was in trouble throughout, and the clock was not pinned. | offered | achieved | p50 | p90 | p99 | errors | cache hit | |---|---|---|---|---|---|---| @@ -68,14 +67,9 @@ Cold start to first response: **3,056 ms** — the first cold number the project What it suggests, pending a run that completes: -- **Capacity is somewhere between 100 and 200 rps**, against 2,000+ rps at `dev` scale on the - same container. A ~13x drop is what a working set exceeding memory looks like, so - `DATA-004` appears to be doing exactly what it was written for. -- **The ladder default brackets this range.** A ladder whose steps all sit past collapse - locates no inflection point, which is the one thing the ladder exists to find. -- **Cache hit rate climbs but stays low** (18% → 33%). At `dev` it reached 77% in the first - step. Worth watching: it may simply be a cold cache over a far larger key space, or the - hot set may be too diffuse at this catalog size even with the skew. +- **Capacity is somewhere between 100 and 200 rps**, against 2,000+ rps at `dev` scale on the same container. A ~13x drop is what a working set exceeding memory looks like, so `DATA-004` appears to be doing exactly what it was written for. +- **The ladder default brackets this range.** A ladder whose steps all sit past collapse locates no inflection point, which is the one thing the ladder exists to find. +- **Cache hit rate climbs but stays low** (18% → 33%). At `dev` it reached 77% in the first step. Worth watching: it may simply be a cold cache over a far larger key space, or the hot set may be too diffuse at this catalog size even with the skew. ## Open @@ -94,6 +88,7 @@ Known gaps, each with why it is still open rather than closed. ## Open decisions +- **Where the tier multipliers live.** They are now normative in `SPEC.md` POST /cart/:id/quote (*Resolved unit price*), which closes the ambiguity. The alternative is `rate` rows discriminated by `kind: 'tier'`: the figures would then be dataset-pinned and checksummed, derivable by any implementation from data it already loads, and the duplicated constant in `resources/lib/pricing.js` and `packages/seed/src/vocabulary.ts` — which nothing currently checks for drift — would go away. Costs one dataset regeneration, which the pricing phase balance decision below likely spends anyway. - **Facet counting**, when listing pages return. Three candidates identified, none chosen; see [`future-work.md`](future-work.md). - **Promotion eligibility lookup** — indexed array probes versus one denormalized key. A P1 measurement, not a guess. See [`data-model.md`](data-model.md). - **Dataset distribution via Git LFS or release assets.** LFS today: the org is on Enterprise with ample headroom and near-zero usage, so the current dataset is comfortable. Release assets are unmetered and deletable, which matters if regeneration becomes routine. Revisit if the dataset grows substantially, regeneration becomes frequent, or clone volume climbs. diff --git a/docs/seed-design.md b/docs/seed-design.md index 7fda6be..b064606 100644 --- a/docs/seed-design.md +++ b/docs/seed-design.md @@ -1,6 +1,6 @@ # Dataset generation -Status: **P0 design.** Implements SPEC.md §3. +Status: **P0 design.** Implements [Data model](../SPEC.md#data-model). ## Generate once, commit the artifact diff --git a/docs/structure.md b/docs/structure.md index 249bbdf..a6ce732 100644 --- a/docs/structure.md +++ b/docs/structure.md @@ -4,36 +4,20 @@ Status: **decided** (P0). Verified against Harper `5.2.12`. ## The question -This repo is an **Example** under the org repository taxonomy: it must be clonable and -deployable as-is, and it tracks the latest Harper release. But it also wants shared -packages — a platform-neutral spec, a seed generator, and a portable e2e suite that runs -against competitor implementations too. +This repo is an **Example** under the org repository taxonomy: it must be clonable and deployable as-is, and it tracks the latest Harper release. But it also wants shared packages — a platform-neutral spec, a seed generator, and a portable e2e suite that runs against competitor implementations too. -Those two pull against each other. The alternative resolution is a `vendor/` directory plus -a bundle step, which works and costs a build stage plus a copy of every shared file. Plain -workspaces avoid both. +Those two pull against each other. The alternative resolution is a `vendor/` directory plus a bundle step, which works and costs a build stage plus a copy of every shared file. Plain workspaces avoid both. ## The constraint (why the app is at the repo root) -**The repo root is the Harper component root.** Not a preference — a hard constraint, from -two independent places: +**The repo root is the Harper component root.** Not a preference — a hard constraint, from two independent places: -1. **Harper does not implement npm's `#path:` git extension.** `deploy_component` with - `package=github:owner/repo` resolves through `parseGitReference` - (`components/Application.ts`), which returns `null` for a `#path:` committish. The caller - treats that as "recognized as git, but not safely handleable" and **fails loudly** rather - than falling back. So `package=github:HarperFast/ecommerce-store#path:apps/store` is not - deployable. A single directory tree is the unit of deployment. -2. Harper framework plugins that wrap an application (`@harperfast/nextjs`, for example) - likewise expect their config at the component root. +1. **Harper does not implement npm's `#path:` git extension.** `deploy_component` with `package=github:owner/repo` resolves through `parseGitReference` (`components/Application.ts`), which returns `null` for a `#path:` committish. The caller treats that as "recognized as git, but not safely handleable" and **fails loudly** rather than falling back. So `package=github:HarperFast/ecommerce-store#path:apps/store` is not deployable. A single directory tree is the unit of deployment. +2. Harper framework plugins that wrap an application (`@harperfast/nextjs`, for example) likewise expect their config at the component root. -Consequence: `config.yaml`, `resources/` and `schemas/` live at the top level, and -`packages/*` are subdirectories of the deployed tree. +Consequence: `config.yaml`, `resources/` and `schemas/` live at the top level, and `packages/*` are subdirectories of the deployed tree. -**Note on (2):** P0 has no web framework — it is two JSON endpoints, and Next.js, React and -a shared component package were removed rather than carried as inert dependencies that ship -~300 MB to every node. Constraint (1) is sufficient on its own, so reintroducing a -framework with the storefront reopens nothing here. +**Note on (2):** P0 has no web framework — it is two JSON endpoints, and Next.js, React and a shared component package were removed rather than carried as inert dependencies that ship ~300 MB to every node. Constraint (1) is sufficient on its own, so reintroducing a framework with the storefront reopens nothing here. ## The resolution: plain npm workspaces, no vendoring, no bundle step @@ -46,68 +30,42 @@ Verified empirically against the exact commands Harper runs, not inferred: | Install | `npm install --force --omit=dev --no-audit --no-fund` (cwd = component root) | `node_modules/@ecommerce-store/{spec,ui,seed}` → relative symlinks into `packages/`; 26 packages, no dev tooling | | Runtime | `import { … } from '@ecommerce-store/spec'` | resolves | -`--install-links` is added only on win32 for candidate builds; on POSIX (Fabric) npm links -`file:`/workspace dependencies **relatively**, which survives the staging→live rename. +`--install-links` is added only on win32 for candidate builds; on POSIX (Fabric) npm links `file:`/workspace dependencies **relatively**, which survives the staging→live rename. ### The rule that matters -> `--omit=dev` at the root does **not** protect you from a workspace package's own -> `dependencies`. They are installed on the node regardless of how the root references that -> workspace. +> `--omit=dev` at the root does **not** protect you from a workspace package's own `dependencies`. They are installed on the node regardless of how the root references that workspace. -Measured: a workspace listed in the root's `devDependencies`, carrying an external package -in its own `dependencies`, **was installed** under `--omit=dev`. Moving that package to the -workspace's `devDependencies` omitted it correctly. +Measured: a workspace listed in the root's `devDependencies`, carrying an external package in its own `dependencies`, **was installed** under `--omit=dev`. Moving that package to the workspace's `devDependencies` omitted it correctly. So: - `packages/spec` — runtime. Root `dependencies`. Keep its deps minimal. -- `packages/seed` — dev-only. Everything heavy goes in **that workspace's** - `devDependencies`, never its `dependencies`. +- `packages/seed` — dev-only. Everything heavy goes in **that workspace's** `devDependencies`, never its `dependencies`. -A dev-only workspace is still symlinked onto the node — that is harmless (source files -only) and it is what lets the deployment double as an E2E target for Harper releases. +A dev-only workspace is still symlinked onto the node — that is harmless (source files only) and it is what lets the deployment double as an E2E target for Harper releases. ### Why `e2e/` is not a workspace -`@ecommerce-store/e2e` lives at the repo root, outside the `packages/*` glob, with its own -`package.json` and its own `node_modules`. It consumes the spec via -`"@ecommerce-store/spec": "file:../packages/spec"`. +`@ecommerce-store/e2e` lives at the repo root, outside the `packages/*` glob, with its own `package.json` and its own `node_modules`. It consumes the spec via `"@ecommerce-store/spec": "file:../packages/spec"`. That looks like an inconsistency. Two reasons it is not. -**Optional peer dependencies defeat `--omit=dev`.** npm installs an optional peer as soon as -anything in the resolution tree makes it satisfiable. A package that declares -`@playwright/test` as an optional peer — web frameworks do — will therefore pull Playwright -onto a deployed node merely because some workspace in the tree declares it, even in that -workspace's `devDependencies`, even under `--omit=dev`. That is tens of megabytes of browser -tooling on a production node, and nothing in the manifest shows it coming. +**Optional peer dependencies defeat `--omit=dev`.** npm installs an optional peer as soon as anything in the resolution tree makes it satisfiable. A package that declares `@playwright/test` as an optional peer — web frameworks do — will therefore pull Playwright onto a deployed node merely because some workspace in the tree declares it, even in that workspace's `devDependencies`, even under `--omit=dev`. That is tens of megabytes of browser tooling on a production node, and nothing in the manifest shows it coming. -Keeping Playwright out of the workspace graph entirely is the only form of this that does -not depend on which packages happen to be installed. +Keeping Playwright out of the workspace graph entirely is the only form of this that does not depend on which packages happen to be installed. -**The e2e suite is not part of this application.** It is the executable spec: it runs -against **any** implementation via `BASE_URL` — Harper, Vercel, Supabase — so it does not -belong in the dependency graph of one of them. +**The e2e suite is not part of this application.** It is the executable spec: it runs against **any** implementation via `BASE_URL` — Harper, Vercel, Supabase — so it does not belong in the dependency graph of one of them. -`npm pack` still ships `e2e/**` as source (kilobytes), so a deployed node remains a -self-describing E2E target; its dependencies are simply never installed there. +`npm pack` still ships `e2e/**` as source (kilobytes), so a deployed node remains a self-describing E2E target; its dependencies are simply never installed there. -**If you add a dev tool anywhere under `packages/*`, re-run the deploy simulation in -`scripts/verify-deploy.sh` and check the installed package count.** Optional peer dependencies -make this class of leak invisible in the manifest. +**If you add a dev tool anywhere under `packages/*`, re-run the deploy simulation in `scripts/verify-deploy.sh` and check the installed package count.** Optional peer dependencies make this class of leak invisible in the manifest. ### Other install-path behavior worth knowing -- If `node_modules` already exists in the payload, **install is skipped entirely** and the - runtime is treated as opaque for redeploy comparison. Never ship `node_modules`. -- **`.npmignore` REPLACES `.gitignore` for packing — it does not merge with it.** Anything `.gitignore` excludes must be repeated there or it lands in the - deployed component. It exists because the ~800 MB dataset is committed to git (it is the - benchmark's contract) but must not ship in a deploy payload: it is loaded separately via - the operations API, outside the measured run. -- `devEngines.packageManager` can select pnpm/yarn, which then run with **their own** - install defaults (dev dependencies included). We stay on npm deliberately: the zero-config - path is the one a customer will clone, and it is the one Harper exercises by default. +- If `node_modules` already exists in the payload, **install is skipped entirely** and the runtime is treated as opaque for redeploy comparison. Never ship `node_modules`. +- **`.npmignore` REPLACES `.gitignore` for packing — it does not merge with it.** Anything `.gitignore` excludes must be repeated there or it lands in the deployed component. It exists because the ~800 MB dataset is committed to git (it is the benchmark's contract) but must not ship in a deploy payload: it is loaded separately via the operations API, outside the measured run. +- `devEngines.packageManager` can select pnpm/yarn, which then run with **their own** install defaults (dev dependencies included). We stay on npm deliberately: the zero-config path is the one a customer will clone, and it is the one Harper exercises by default. - A custom `install.command` makes the whole installation opaque to Harper. Avoided. ## Layout @@ -131,23 +89,12 @@ ecommerce-store/ repo root = Harper component root ### Why the harness lives here -`bench/` and `containers/` are measurement scaffolding, and scaffolding shared across -comparisons belongs in the benchmarks repo. It is here because there is one implementation, -so there is nothing yet to share it *with*, and a harness developed apart from the only -target it runs against is a harness nobody runs. +`bench/` and `containers/` are measurement scaffolding, and scaffolding shared across comparisons belongs in the benchmarks repo. It is here because there is one implementation, so there is nothing yet to share it *with*, and a harness developed apart from the only target it runs against is a harness nobody runs. -It moves to the benchmarks repo when a second target makes it genuinely shared — which is -the same point at which "containerized identically for every target" stops being vacuous. +It moves to the benchmarks repo when a second target makes it genuinely shared — which is the same point at which "containerized identically for every target" stops being vacuous. -Until then the separation is by directory, not by repo: nothing under `resources/` or -`packages/` imports from `bench/`, and `.npmignore` keeps both out of a deployed component. -A customer reading this reference should see an ecommerce application, not a benchmark rig. +Until then the separation is by directory, not by repo: nothing under `resources/` or `packages/` imports from `bench/`, and `.npmignore` keeps both out of a deployed component. A customer reading this reference should see an ecommerce application, not a benchmark rig. ## Open -- **`server-timing` is emitted inline, not by a plugin.** Both endpoints set the header - directly, which satisfies `OBS-001`/`OBS-003` and needs no plugin ordering. The commented - block in `config.yaml` records the ordering constraint for the day a plugin does the - decomposition — earlier-registered http listeners are outer middleware layers, so it - would have to be registered before any handler whose time it decomposes. Left commented - rather than deleted because the constraint is the expensive part to rediscover. +- **`server-timing` is emitted inline, not by a plugin.** Both endpoints set the header directly, which satisfies `OBS-001`/`OBS-003` and needs no plugin ordering. The commented block in `config.yaml` records the ordering constraint for the day a plugin does the decomposition — earlier-registered http listeners are outer middleware layers, so it would have to be registered before any handler whose time it decomposes. Left commented rather than deleted because the constraint is the expensive part to rediscover. diff --git a/e2e/lib/spec.ts b/e2e/lib/spec.ts index d051eb4..f404be6 100644 --- a/e2e/lib/spec.ts +++ b/e2e/lib/spec.ts @@ -3,7 +3,7 @@ * * Every test names what it covers, as `covers('QUOTE-005')`, which prefixes the test title * with those ids. `scripts/coverage.mjs` then scans the suite and fails if any MUST has no - * test — SPEC.md §8. + * test — SPEC.md#conformance. * * Using the title as the carrier (rather than a runtime registry) keeps coverage checkable * statically, without running the suite against a live implementation. diff --git a/e2e/scripts/coverage.mjs b/e2e/scripts/coverage.mjs index ced193d..6a196a1 100644 --- a/e2e/scripts/coverage.mjs +++ b/e2e/scripts/coverage.mjs @@ -1,5 +1,5 @@ /** - * Assert that every MUST in SPEC.md is covered by at least one test — SPEC.md §8. + * Assert that every MUST in SPEC.md is covered by at least one test — SPEC.md#conformance. * * Static: scans test titles for the `[REQ-ID ...]` prefix that `covers()` emits, so it runs * without a live implementation. Exits non-zero on a gap. @@ -47,8 +47,6 @@ const active = activeRequirements(); const musts = active.filter((r) => r.level === 'MUST'); const gaps = coverageGaps(covered); -console.log(`${active.length} active requirements, ${musts.length} MUST`); -console.log(`covered by tests: ${covered.size}`); if (gaps.length) { console.error(`\nFAIL — ${gaps.length} MUST requirement(s) have no test:`); diff --git a/e2e/tests/cache.api.spec.ts b/e2e/tests/cache.api.spec.ts index 15620ec..98b2923 100644 --- a/e2e/tests/cache.api.spec.ts +++ b/e2e/tests/cache.api.spec.ts @@ -1,4 +1,4 @@ -/** Caching — SPEC.md §3. Derived, bounded, correctly keyed. */ +/** Caching — SPEC.md#data-model. Derived, bounded, correctly keyed. */ import { test, expect } from '@playwright/test'; import { API, productSearchParams } from '@ecommerce-store/spec'; import { covers } from '../lib/spec.ts'; @@ -40,7 +40,8 @@ test.describe('caching', () => { const write = await request.post(`/admin/variant/${sku}`, { data: { basePrice: 54321 } }); expect(write.ok()).toBe(true); - // The fan-out is the cost SPEC.md §6 measures, so the write reports what it invalidated. + // The fan-out is the cost SPEC.md#background-writes measures, so the write reports what it + // invalidated. expect((await write.json()).invalidated).toBeGreaterThan(0); const after = await request.get(url); diff --git a/e2e/tests/platform.api.spec.ts b/e2e/tests/platform.api.spec.ts index f377cf6..0145332 100644 --- a/e2e/tests/platform.api.spec.ts +++ b/e2e/tests/platform.api.spec.ts @@ -1,5 +1,5 @@ /** - * Background writes and the remaining quote instrumentation — SPEC.md §6, §7. + * Background writes and the remaining quote instrumentation — SPEC.md#background-writes and SPEC.md#observability. * * The dataset requirements (DATA-001/002/003/005) and the run-level ones (DATA-004, * WRITE-001/003/004) are NOT here, deliberately. They are properties of the committed @@ -12,7 +12,7 @@ import { test, expect } from '@playwright/test'; import { API } from '@ecommerce-store/spec'; import { covers } from '../lib/spec.ts'; -/** SPEC.md §6. The coherence BUDGET, not the expected latency. */ +/** SPEC.md#background-writes. The coherence BUDGET, not the expected latency. */ const FRESH_MS = 1000; test.describe('background writes', () => { diff --git a/e2e/tests/product.api.spec.ts b/e2e/tests/product.api.spec.ts index 0f13048..6ae6272 100644 --- a/e2e/tests/product.api.spec.ts +++ b/e2e/tests/product.api.spec.ts @@ -1,9 +1,9 @@ -/** Product aggregate — SPEC.md §5. The read-heavy leg. */ +/** Product aggregate — SPEC.md#get-productidtierregion. The read-heavy leg. */ import { test, expect } from '@playwright/test'; import { API, productSearchParams } from '@ecommerce-store/spec'; import { covers } from '../lib/spec.ts'; -/** SPEC.md §6. The coherence BUDGET, not the expected latency. */ +/** SPEC.md#background-writes. The coherence BUDGET, not the expected latency. */ const FRESH_MS = 1000; const get = (id: string, tier: string, region: string) => `${API.product(id)}?${productSearchParams({ tier, region })}`; diff --git a/e2e/tests/quote.api.spec.ts b/e2e/tests/quote.api.spec.ts index 555077f..d6660d3 100644 --- a/e2e/tests/quote.api.spec.ts +++ b/e2e/tests/quote.api.spec.ts @@ -1,5 +1,5 @@ /** - * Cart quote — SPEC.md §4. The primary endpoint under test. + * Cart quote — the primary endpoint under test. SPEC.md#post-cartidquote. * * P0 skeleton: titles bind to requirement ids and the shapes are stated; assertions are * fixme until P1 puts data behind them. Landing it now means the coverage check is real @@ -11,7 +11,7 @@ import { covers } from '../lib/spec.ts'; test.describe('cart quote', () => { test(covers('QUOTE-001')('two carts with identical contents are priced independently'), async ({ request }) => { - // Response-level caching is a conformance failure (SPEC.md §4), and the signature of it + // Response-level caching is a conformance failure (SPEC.md#post-cartidquote), and the signature of it // is a response whose cartId does not match the cart asked for. A repeat request must // also re-do the work rather than return a stored body under a different key. const ids = Array.from({ length: 24 }, (_, i) => `cart-${String(i * 31).padStart(6, '0')}`); @@ -201,7 +201,7 @@ test.describe('cart quote', () => { checked++; const body = await response.json(); const lineSum = body.lines.reduce((sum: number, l: { lineTotal: number }) => sum + l.lineTotal, 0); - // The cap is FLOORED (SPEC.md §4): asserting only `<= subtotal` accepted a 90% + // The cap is FLOORED (SPEC.md#promotion-evaluation-order step 9): asserting only `<= subtotal` accepted a 90% // discount, which is what the previous version of this test did. const cap = Math.floor((body.subtotal * 6000) / 10000); if (body.discountTotal > cap || body.discountTotal < 0 || body.subtotal !== lineSum) { diff --git a/package.json b/package.json index 2f14269..ed9f5c3 100644 --- a/package.json +++ b/package.json @@ -17,8 +17,9 @@ "seed": "node packages/seed/src/cli.ts", "typecheck": "tsc --build", "test:e2e": "npm test --prefix e2e", - "check": "npm run check:spec && npm run check:coverage && npm run check:index && npm run check:dataset && npm run typecheck && npm run check:deploy", + "check": "npm run check:spec && npm run check:anchors && npm run check:coverage && npm run check:index && npm run check:dataset && npm run typecheck && npm run check:deploy", "check:spec": "node scripts/check-spec-sync.mjs", + "check:anchors": "node scripts/check-spec-anchors.mjs", "check:coverage": "npm run coverage --prefix e2e", "check:deploy": "./scripts/verify-deploy.sh", "check:index": "node scripts/verify-promotion-index.mjs --scale dev", diff --git a/packages/seed/src/generate.ts b/packages/seed/src/generate.ts index 96340d8..496d58e 100644 --- a/packages/seed/src/generate.ts +++ b/packages/seed/src/generate.ts @@ -1,5 +1,5 @@ /** - * The dataset generator — SPEC.md §3, docs/seed-design.md. + * The dataset generator — SPEC.md#data-model, docs/seed-design.md. * * Yields rows table by table so the output streams: the benchmark dataset is deliberately * larger than memory, and holding it all before writing would force every implementation to diff --git a/packages/seed/src/index.ts b/packages/seed/src/index.ts index 5c18816..0b40b88 100644 --- a/packages/seed/src/index.ts +++ b/packages/seed/src/index.ts @@ -1,5 +1,5 @@ /** - * Dataset generation — SPEC.md §3, designed in docs/seed-design.md. + * Dataset generation — SPEC.md#data-model, designed in docs/seed-design.md. * * P0 declares the contract; the generator itself is P1. * diff --git a/packages/spec/src/model.ts b/packages/spec/src/model.ts index 7afc671..43c74ef 100644 --- a/packages/spec/src/model.ts +++ b/packages/spec/src/model.ts @@ -1,5 +1,5 @@ /** - * Domain model — SPEC.md §3. + * Domain model — SPEC.md#data-model. * * Stack-neutral. These types describe what crosses the wire, never how anything is stored. * If a type here could not be produced by Fastify + Postgres + Redis, it is mis-specified. @@ -33,7 +33,7 @@ export interface Product { categoryIds: string[]; /** Grams. Summed across lines for shipping — QUOTE-006. */ weight: number; - /** Fan out to further product reads — see the §3 foldings. */ + /** Fan out to further product reads — see SPEC.md#foldings-and-what-they-cost. */ relatedProductIds: string[]; reviewRollup: ReviewRollup; } @@ -125,7 +125,7 @@ export interface Quote { currency: string; } -/** SPEC.md §5. Varies by tier and region — PDP-002. */ +/** SPEC.md#get-productidtierregion. Varies by tier and region — PDP-002. */ export interface ProductAggregate { product: Product; variants: Variant[]; diff --git a/packages/spec/src/requirements.ts b/packages/spec/src/requirements.ts index c993ca8..62f7370 100644 --- a/packages/spec/src/requirements.ts +++ b/packages/spec/src/requirements.ts @@ -2,7 +2,7 @@ * The requirement registry — the machine-readable index of SPEC.md. * * Every test in the verification suite names the requirement ids it covers, and every MUST - * must be covered by at least one test (SPEC.md §8). `coverageGaps()` makes that checkable + * must be covered by at least one test (SPEC.md#conformance). `coverageGaps()` makes that checkable * rather than aspirational. * * Ids are PERMANENT. Withdraw a requirement by setting `withdrawn: true`; never reuse an id. @@ -23,19 +23,19 @@ export interface Requirement { const r = (id: string, area: Area, level: Level, summary: string): Requirement => ({ id, area, level, summary }); export const REQUIREMENTS: readonly Requirement[] = [ - // §3 Data model + // Data model r('DATA-001', 'DATA', 'MUST', 'Exactly the eight logical entities; a differing representation is documented, never pre-joined'), r('DATA-002', 'DATA', 'MUST', 'Money is integer minor units end to end'), r('DATA-003', 'DATA', 'MUST', 'Every implementation loads the same versioned dataset, checksum-verified'), r('DATA-004', 'DATA', 'MUST', 'One dataset size, with a working set that does not fit in memory'), r('DATA-005', 'DATA', 'MUST', 'The dataset is generated deterministically and version controlled'), - // §3 Caching — derived, bounded, correctly keyed. + // Caching — derived, bounded, correctly keyed. r('CACHE-001', 'CACHE', 'MUST', 'Caches are derived: dropping every cache changes no response body, only latency'), r('CACHE-002', 'CACHE', 'MUST', 'Every cached value is bounded by FRESH_MS, by invalidation or expiry, and says which'), r('CACHE-003', 'CACHE', 'MUST', 'A cache key includes every dimension the cached value varies by'), - // §4 Cart quote + // POST /cart/:id/quote r('QUOTE-001', 'QUOTE', 'MUST', 'Every response is unique to its cart; response-level caching is a conformance failure'), r('QUOTE-002', 'QUOTE', 'MUST', 'Each line resolves product and variant; an unknown sku fails the quote with 400'), r('QUOTE-003', 'QUOTE', 'MUST', 'Availability resolves against inventory across locations, honouring priority'), @@ -49,20 +49,20 @@ export const REQUIREMENTS: readonly Requirement[] = [ r('QUOTE-011', 'QUOTE', 'MUST', 'Cart-wide and BOGO promotions apply once per cart, stackables once per eligible line (max 3); discountTotal in [0, 60% of subtotal]'), r('QUOTE-012', 'QUOTE', 'MUST', 'appliedPromotionIds lists exactly the promotions that discounted that line'), - // §5 Product aggregate + // GET /product/:id r('PDP-001', 'PDP', 'MUST', 'Aggregates product, variants, inventory, resolved price, review rollup and related items'), r('PDP-002', 'PDP', 'MUST', 'Price varies by tier and availability by region; both are in any cache key'), r('PDP-003', 'PDP', 'MUST', 'Inventory and price reflect background writes within FRESH_MS'), r('PDP-004', 'PDP', 'MUST', 'An unknown product id returns 404'), r('PDP-005', 'PDP', 'SHOULD', 'Related items carry enough detail to render without a further request'), - // §6 Background writes + // Background writes r('WRITE-001', 'WRITE', 'MUST', 'A steady writer updates inventory and prices within the read working set'), r('WRITE-002', 'WRITE', 'MUST', 'A committed write is observable on the aggregate and in quote pricing within FRESH_MS'), r('WRITE-003', 'WRITE', 'MUST', 'Coherence is not satisfied by disabling caching; cache-hit rates are recorded'), r('WRITE-004', 'WRITE', 'MUST', 'The write stream is identical in rate and key distribution for every target'), - // §7 Observability + // Observability r('OBS-001', 'OBS', 'MUST', 'Both endpoints emit Server-Timing decomposing data access, compute and total'), r('OBS-002', 'OBS', 'MUST', 'Responses indicate cache status and the mechanism is documented'), r('OBS-003', 'OBS', 'MUST', 'Instrumentation exists from the first commit of a surface'), diff --git a/packages/spec/src/routes.ts b/packages/spec/src/routes.ts index 28d85ec..843d6d3 100644 --- a/packages/spec/src/routes.ts +++ b/packages/spec/src/routes.ts @@ -1,5 +1,5 @@ /** - * The route contract — SPEC.md §4, §5. + * The route contract — SPEC.md#post-cartidquote and SPEC.md#get-productidtierregion. * * Canonical paths. An implementation whose stack idiom differs MAY serve equivalent paths * and MUST then publish a mapping the verification suite is configured with. @@ -29,7 +29,7 @@ export function productSearchParams({ tier, region }: ProductQuery): URLSearchPa return params; } -/** Promotion evaluation order — SPEC.md §4. Normative: an unspecified order makes two correct implementations disagree on a total. */ +/** Promotion evaluation order — SPEC.md#post-cartidquote. Normative: an unspecified order makes two correct implementations disagree on a total. */ export const PROMOTION_ORDER = ['exclusive', 'threshold', 'bogo', 'stackable'] as const; /** A non-canonical implementation declares its paths here; published with the comparison. */ diff --git a/resources/admin.js b/resources/admin.js index be5b09b..2cdd5e6 100644 --- a/resources/admin.js +++ b/resources/admin.js @@ -1,16 +1,16 @@ /** - * The write surface — SPEC.md §6. + * The write surface — SPEC.md#background-writes. * * The background writer drives these. It used to write straight to the tables through the * operations API, which worked but could never invalidate anything: the cache would only - * converge on expiry, and §6's whole purpose is to make cache coherence a cost the + * converge on expiry, and Background writes's whole purpose is to make cache coherence a cost the * architecture actually pays. * * Writing through the application is also what a separated stack must do — its write path * updates Postgres and then evicts the Redis key. The fan-out below is the Harper side of * exactly that comparison. * - * Not an endpoint under test: its own latency is not a headline metric (SPEC.md §6). + * Not an endpoint under test: its own latency is not a headline metric (SPEC.md#background-writes). */ import { Resource } from 'harper'; import { REGIONS, TIERS, viewKey } from './product.js'; diff --git a/resources/lib/pricing.js b/resources/lib/pricing.js index 7f44d76..5562439 100644 --- a/resources/lib/pricing.js +++ b/resources/lib/pricing.js @@ -1,5 +1,5 @@ /** - * The pricing engine — SPEC.md §4. + * The pricing engine — SPEC.md#resolved-unit-price and SPEC.md#promotion-evaluation-order. * * Pure: no I/O, no clock, no randomness. Everything it needs is passed in, which is what * makes QUOTE-008 (byte-identical quotes for the same cart and dataset state) testable @@ -18,7 +18,7 @@ /** Tier multipliers in basis points. Fixed, not drawn — must match packages/seed. */ const TIER_BASIS_POINTS = { standard: 10000, silver: 9500, gold: 9000, platinum: 8500 }; -/** Round half-up to the minor unit. Applied per discount, not once at the end (SPEC.md §4). */ +/** Round half-up to the minor unit. Applied per discount, not once at the end — SPEC.md#promotion-evaluation-order. */ function applyBasisPoints(amount, basisPoints) { return Math.floor((amount * basisPoints + 5000) / 10000); } @@ -64,7 +64,7 @@ function meetsThreshold(promotion, subtotal) { const byPromotionId = (a, b) => (a.id < b.id ? -1 : a.id > b.id ? 1 : 0); /** - * Stacking limits — SPEC.md §4. + * Stacking limits — SPEC.md#promotion-evaluation-order steps 8 and 9. * * Real stores bound stacking; without a bound, a corpus with enough unrestricted stackables * compounds a cart toward zero and the benchmark prices free carts. These are normative so @@ -93,7 +93,7 @@ function groupByPromotion(candidates) { } /** - * Evaluate promotions in the normative order — SPEC.md §4. + * Evaluate promotions in the normative order — SPEC.md#promotion-evaluation-order. * * The order is normative because stacking is order-dependent: without it two correct * implementations disagree on a total, which the measurement rules classify as @@ -106,11 +106,11 @@ function groupByPromotion(candidates) { * * Ties break on ascending promotion id at every step. * - * Two accumulators, because cart-wide and per-line discounts cannot share one budget without - * one silently eating the other's headroom: - * - `cartDiscount` for exclusive/threshold, capped at the subtotal - * - per-line `remaining`, which bogo/stackable draw down, so a line can never be discounted - * below zero and a percentage promotion compounds against what is actually left + * ONE budget: the per-line `remaining` map, which every phase draws down through `drawDown`. + * Cart-wide discounts are allocated across the lines they are eligible for in proportion to + * what each still has left. A separate cart-level accumulator alongside these lets both + * discount the same money — the cap bounds the total either way, so the double-spend never + * shows in `discountTotal`, only in per-line figures that stop summing to it. */ export function evaluatePromotions({ lines: inputLines, candidates, subtotal }) { /** @@ -222,7 +222,10 @@ export function evaluatePromotions({ lines: inputLines, candidates, subtotal }) .filter((l) => l && l.quantity >= 2 && (remaining.get(l.sku) ?? 0) > 0) .sort((a, b) => a.unitPrice - b.unitPrice || (a.sku < b.sku ? -1 : 1))[0]; if (!line) continue; - const touched = drawDown([line.sku], line.unitPrice); + // The promotion's own amount, against that ONE unit — not the line (SPEC.md#promotion-evaluation-order step 4). + // The corpus carries 10000 basis points on every bogo row, which makes the unit free; + // reading the magnitude rather than assuming it keeps a corpus that says otherwise correct. + const touched = drawDown([line.sku], discountFor(entry.promotion, line.unitPrice)); if (touched.length) attribute(entry, touched); } diff --git a/resources/product.js b/resources/product.js index 51730b8..ac5d6f0 100644 --- a/resources/product.js +++ b/resources/product.js @@ -1,5 +1,5 @@ /** - * GET /product/:id?tier=®ion= — SPEC.md §5. The read-heavy leg. + * GET /product/:id?tier=®ion= — SPEC.md#get-productidtierregion. The read-heavy leg. * * Fans out: product, its variants, inventory for those variants, then related products. * The fan-out is deliberate (DATA-001) — a pre-joined source of truth would remove the reads @@ -9,7 +9,7 @@ * `@table(expiration)` cache whose `sourcedFrom` resolver below does the assembly; a hit * returns without touching any of the eight entities. This is the Harper-native form of what * a separated stack does with Redis, and the comparison is about what keeping it coherent - * costs, not about who remembered to cache (SPEC.md §3 Caching). + * costs, not about who remembered to cache (SPEC.md#caching). */ import { Resource } from 'harper'; import { resolveUnitPrice } from './lib/pricing.js'; diff --git a/resources/quote.js b/resources/quote.js index 10be31d..e809d5e 100644 --- a/resources/quote.js +++ b/resources/quote.js @@ -1,5 +1,5 @@ /** - * POST /cart/:id/quote — SPEC.md §4. The primary endpoint under test. + * POST /cart/:id/quote — the primary endpoint under test. SPEC.md#post-cartidquote. * * Four to five dependent waves, 60-150 record reads, real pricing logic. Every response is * unique to its cart, so no implementation can win here by caching a response (QUOTE-001). diff --git a/schemas/store.graphql b/schemas/store.graphql index 419af78..a1d8399 100644 --- a/schemas/store.graphql +++ b/schemas/store.graphql @@ -1,4 +1,4 @@ -# The eight tables — SPEC.md §3, designed in docs/data-model.md. +# The eight tables — SPEC.md#data-model, designed in docs/data-model.md. # # Deliberately NOT pre-joined. The fan-out across these tables is what the benchmark # measures (DATA-001), so a shape that collapses a required read is a conformance failure, @@ -28,7 +28,7 @@ type Product @table @export @sealed { title: String categoryIds: [String] @indexed weight: Int - # Fans out to further Product reads — depth preserved, see docs/data-model.md §foldings. + # Fans out to further Product reads — depth preserved. SPEC.md#foldings-and-what-they-cost. relatedProductIds: [String] # { count, averageCentistars } — a field, not a table. Nothing in P0 writes reviews. reviewRollup: Any @@ -91,7 +91,7 @@ type Rate @table @export @sealed { # --------------------------------------------------------------------------------------- # The cache. # -# GET /product/:id is "mostly cacheable, varies by tier and region" (SPEC.md §5), so the +# GET /product/:id is "mostly cacheable, varies by tier and region" (SPEC.md#get-productidtierregion), so the # cached unit is the fully-resolved aggregate keyed by `${productId}:${tier}:${region}` — # 16 entries per product across 4 tiers and 4 regions. # @@ -99,7 +99,7 @@ type Rate @table @export @sealed { # coherent two ways: explicit invalidation on every write that can change it, with the TTL # as the backstop that bounds staleness even if an invalidation is ever missed. An # implementation on a separated stack puts the same aggregate in Redis with the same key; -# what differs is what invalidating it costs, which is exactly what SPEC.md §6 measures. +# what differs is what invalidating it costs, which is exactly what SPEC.md#background-writes measures. # # NOT @export'ed: it is an internal cache, not a public surface. Reads go through # /product/:id so cache status can be reported (OBS-002). diff --git a/scripts/check-spec-anchors.mjs b/scripts/check-spec-anchors.mjs new file mode 100644 index 0000000..7a63989 --- /dev/null +++ b/scripts/check-spec-anchors.mjs @@ -0,0 +1,88 @@ +/** + * Every reference to a section of SPEC.md must resolve to a heading that exists. + * + * References are written as anchors: a bare `SPEC.md#heading-anchor` in code comments, and a + * markdown link to the same in documents — rather than as section numbers or quoted heading + * text. An anchor is checkable and prose is not, which is how a + * reference to a `#foldings` section of docs/data-model.md survived in schemas/store.graphql + * long after that discussion had moved into SPEC.md. + * + * Renaming a heading now fails here instead of silently orphaning every pointer at it. + * + * node scripts/check-spec-anchors.mjs + */ +import { readFileSync, readdirSync, statSync } from 'node:fs'; +import { join, relative, extname, dirname, resolve } from 'node:path'; + +const ROOT = new URL('..', import.meta.url).pathname; +const SPEC = 'SPEC.md'; + +/** GitHub's heading-anchor algorithm: lowercase, drop punctuation, spaces to hyphens. */ +function anchorOf(heading) { + return heading + .trim() + .toLowerCase() + .replace(/[^\w\s-]/g, '') + .replace(/\s+/g, '-') + .replace(/^-+|-+$/g, ''); +} + +const spec = readFileSync(join(ROOT, SPEC), 'utf8'); +const headings = [...spec.matchAll(/^#{1,6} +(.+?)\s*$/gm)].map((m) => m[1]); +const anchors = new Set(headings.map(anchorOf)); + +const SKIP = new Set(['node_modules', '.git', '.work', 'dataset', 'test-results', 'playwright-report']); +const EXT = new Set(['.md', '.js', '.mjs', '.ts', '.tsx', '.graphql', '.yaml', '.yml', '.sh']); + +function* files(dir) { + for (const name of readdirSync(dir)) { + if (SKIP.has(name)) continue; + const path = join(dir, name); + if (statSync(path).isDirectory()) yield* files(path); + else if (EXT.has(extname(name))) yield path; + } +} + +const problems = []; + +// This file names anchors in its own documentation and error messages. It is the checker, +// not a reference, so it does not check itself. +const SELF = relative(ROOT, new URL(import.meta.url).pathname); + +for (const path of files(ROOT)) { + const rel = relative(ROOT, path); + if (rel === SELF) continue; + const source = readFileSync(path, 'utf8'); + const lines = source.split('\n'); + + lines.forEach((line, i) => { + // Cross-file references from anywhere, plus same-document links inside SPEC.md itself. + const refs = [...line.matchAll(/SPEC\.md#([\w-]+)/g)].map((m) => m[1]); + if (rel === SPEC) refs.push(...[...line.matchAll(/\]\(#([\w-]+)\)/g)].map((m) => m[1])); + + for (const ref of refs) { + if (!anchors.has(ref)) problems.push(`${rel}:${i + 1} SPEC.md#${ref} — no such heading`); + } + + // A markdown link also has to point at SPEC.md from where it is written. The anchor can be + // perfect and the path still wrong, which is how every link in docs/ pointed at a sibling + // SPEC.md that is actually one directory up. + for (const [, target] of line.matchAll(/\]\((\.{0,2}[\w./-]*SPEC\.md)#[\w-]+\)/g)) { + const resolved = resolve(dirname(path), target); + if (resolved !== join(ROOT, SPEC)) { + const correct = relative(dirname(path), join(ROOT, SPEC)) || SPEC; + problems.push(`${rel}:${i + 1} links to ${target} — from here that path is not ${SPEC}; use ${correct}`); + } + } + }); +} + +if (problems.length) { + console.error('FAIL — SPEC.md section references that resolve to nothing:'); + for (const p of problems) console.error(` ${p}`); + console.error('\nValid anchors:'); + for (const a of [...anchors].sort()) console.error(` #${a}`); + process.exit(1); +} + +console.log('OK — every SPEC.md section reference resolves.'); diff --git a/scripts/check-spec-sync.mjs b/scripts/check-spec-sync.mjs index 85260da..89e6917 100644 --- a/scripts/check-spec-sync.mjs +++ b/scripts/check-spec-sync.mjs @@ -37,9 +37,8 @@ for (const req of REQUIREMENTS) { } } -console.log(`SPEC.md: ${inSpec.size} ids · registry: ${inRegistry.size} ids`); if (problems.length) { - console.error(`\nFAIL — ${problems.length} inconsistency(ies):`); + console.error('FAIL — SPEC.md and the registry disagree:'); for (const p of problems) console.error(` ${p}`); process.exit(1); } diff --git a/scripts/verify-dataset.mjs b/scripts/verify-dataset.mjs index 2a90f3a..25996c9 100644 --- a/scripts/verify-dataset.mjs +++ b/scripts/verify-dataset.mjs @@ -103,8 +103,8 @@ try { failures.push('DATA-005: the generator is not committed — a dataset nobody can regenerate is a magic file'); } -console.log(`dataset ${scale}: seed ${manifest.seed}, generator ${manifest.generatorVersion}, ${Object.keys(manifest.files).length} tables`); -console.log(` ${Object.values(manifest.files).reduce((n, f) => n + f.rows, 0).toLocaleString()} rows, checksums over uncompressed bytes`); +console.log(`dataset ${scale}: seed ${manifest.seed}, generator ${manifest.generatorVersion}`); +console.log(' checksums verified over uncompressed bytes'); if (failures.length) { console.error(`\nFAIL — ${failures.length} dataset requirement(s) not met:`); diff --git a/scripts/verify-deploy.sh b/scripts/verify-deploy.sh index c828d37..57f9af9 100755 --- a/scripts/verify-deploy.sh +++ b/scripts/verify-deploy.sh @@ -31,9 +31,6 @@ echo "==> installing as Harper does" cd node npm install --force --omit=dev --no-audit --no-fund >/dev/null 2>&1 -COUNT="$(ls node_modules 2>/dev/null | grep -vc '^\.' || true)" -echo "==> $COUNT top-level entries in node_modules" - status=0 for pkg in "${FORBIDDEN[@]}"; do if [ -e "node_modules/$pkg" ]; then @@ -52,6 +49,6 @@ for pkg in @ecommerce-store/spec; do done if [ "$status" -eq 0 ]; then - echo "OK: node tree is clean ($(du -sh node_modules | cut -f1))" + echo "OK: nothing dev-only reached the node" fi exit "$status" diff --git a/scripts/verify-promotion-index.mjs b/scripts/verify-promotion-index.mjs index b95ef0a..b05a82b 100644 --- a/scripts/verify-promotion-index.mjs +++ b/scripts/verify-promotion-index.mjs @@ -77,8 +77,7 @@ for (const tier of TIERS) { } } -console.log(`scale ${scale}: ${promotions.length} promotions, ${checks} tier x category combinations checked`); -console.log(`mean candidates per probe: ${(candidateTotal / checks).toFixed(0)} of ${promotions.length} (${((candidateTotal / checks / promotions.length) * 100).toFixed(1)}%)`); +console.log(`mean probe selectivity: ${((candidateTotal / checks / promotions.length) * 100).toFixed(1)}% of the corpus`); if (missed) { console.error(`\nFAIL — the index excluded ${missed} eligible promotion(s):`);