# Finalize and independently review content-model Task 17

## Objective

Starting from clean implementation commit `5becd9e8`, close Task 17 only: eliminate every fixable own-repository warning from the content build without suppression, independently review the complete implementation range `33e7a1ba..HEAD`, repair every P1/P2 against the content, fields, program, coding, and authz contracts, rerun the focused Node 22 quality gates warning-free, obtain a final independent `gpt-5.6-sol/low` P1/P2-none review, and commit the scoped result.

The planner artifact `specs/c48f09a5_task-17-final-review.md` is expected process state and may be included in the final commit. Do not edit existing specs or plan ledgers.

## Sources of truth

Read before editing, in this order:

1. `docs/specs/2026-08-08-content-types-complete-design.md`, especially normalized/final stores, migration ordering, transaction capability, structural errors, and D1/Postgres acceptance.
2. `docs/specs/2026-08-08-fields-complete-design.md`, especially §2, §4–§7, §9, §15, and §17–§19: the complete recursive field/group contract, immutable `FieldDefinitionVersion`, explicit extension registries, trust-boundary bounds, parity, and Task 7 migration floor.
3. `docs/specs/2026-08-08-mod-cms-content-model-completion-design.md` for the package boundary, one callback-minted transaction identity, structural errors, and sequencing.
4. `docs/plans/2026-08-08-mod-cms-content-model-completion.md`, Task 17, Task 7, completion/review requirements.
5. `docs/standards/coding-standard.md` §2 and §4 and `docs/standards/authz.md`.
6. The prior ADW artifacts `specs/17368f15_mod-cms-completion.md` and `specs/838b4573_fields-definition-version.md` as historical implementation intent; canonical docs above win on any conflict.

## Fixed constraints

- Use `PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH` for **every** Node or pnpm command.
- Treat every warning from owned source/build configuration as blocking. Fix the cause; do not suppress, allowlist, filter, redirect away, disable tree-shaking, or merely make warning text invisible.
- Do not modify `apps/mod-cms`, `../mod-cms`, distro work, Posts-list work, any spec except this ADW artifact, or unrelated packages/files.
- Do not start Task 2 or Task 7 migrations. Task 17 freezes the contract that Task 7 will consume.
- Preserve the genuine D1 and embedded real-Postgres fixtures; do not replace either with static rows, mocks, skipped tests, or an in-memory stand-in mislabeled as Postgres.
- Preserve all pre-Task-17 public root exports while adding/fixing Task 17 exports. Do not expose internal parser helpers merely to satisfy a test.
- Judge every command by its exit status and inspect successful build output separately for warnings; a zero exit with a warning is not success.

## 1. Baseline and inventory

1. Confirm `git rev-parse HEAD` is `5becd9e8`, inspect `git status --short --branch`, and retain only the new planner artifact as expected process state. Do not reset or rewrite the five existing implementation commits from `33e7a1ba` through `5becd9e8`.
2. Inventory the full range with `git diff --stat`, `git diff --name-status`, and focused diffs for `packages/content/**`, `packages/fields/**`, package manifests, and lockfile. Compare both root barrels to `33e7a1ba` so runtime and type-only exports that existed before Task 17 remain reachable under their intended names.
3. Reproduce the content build with the required Node 22 PATH and retain the unfiltered output. The known blockers are generated `feed.js`, `index.js`, and `search-sqlite.js` reporting unused named imports from `drizzle-orm` and `@platform-modules/db`. Confirm the exact current warning set before changing code.
4. Read the source/module graph that feeds each warned entry: at minimum `packages/content/src/store.ts`, `feed.ts`, `search-sqlite.ts`, `revisions.ts`, `taxonomy.ts`, `index.ts`, and `tsup.config.ts`. Establish which imported bindings are required by other exports in the source module but become unused after per-entry Rollup tree-shaking. Do not mistake an actually dead source import for a bundler-only cross-entry artifact.

## 2. Add discriminating RED coverage for review findings

Before each behavioral repair, add the smallest focused test/type assertion that fails for the defect and passes only for the contract:

- Extend `packages/content/src/conformance.test.ts` and `packages/fields/src/conformance.test.ts` only for adapter/store/transaction defects. Keep tests `.test.ts`.
- Extend `packages/content/src/index.test.ts` and `packages/fields/src/index.test.ts` for public runtime inventory and compile-time root-import checks.
- Extend `packages/fields/src/definition-version.test.ts` for immutable-definition parser/type defects.
- Add no broad duplicate suite and no generated-dist snapshot whose only purpose is matching warning wording.

Run each narrow test to establish RED before its implementation fix, then rerun it immediately after the fix.

## 3. Surgically remove content build warnings

Fix owned code rather than tsup diagnostics:

1. Remove truly dead imports where the binding has no live source use.
2. For bindings that are live in one function but appear unused in another independently bundled entry, make the module boundary/import shape accurately reflect each entry’s live dependency set. Prefer a narrow source-level import refactor or a cohesive extraction of the Task 17 flat-store/transaction capability code over global build changes. A namespace import is acceptable only if it is the smallest semantically honest fix and emitted bundles still tree-shake correctly.
3. Keep `@platform-modules/db` and `drizzle-orm` external and preserve `sideEffects:false`, ESM exports, DTS output, and every existing subpath. Do not change `treeshake`, add Rollup warning handlers, add an allowlist, or toggle splitting merely to silence diagnostics.
4. Restrict edits to `packages/content/src/**` and, only if a real package-local build topology correction is unavoidable, `packages/content/tsup.config.ts`. Do not alter shared `tooling/tsup-config` for one package.
5. Build content again and inspect all ESM and DTS phases. Success is zero owned warnings from every generated entry, with all declared outputs still emitted and importable.

## 4. Independently audit and repair complete Task 17 behavior

Review actual code and tests across `33e7a1ba..HEAD`, not only the latest commit. Fix every P1/P2 found in these areas.

### A. Genuine adapter execution and parity

- D1 must create and seed the real legacy table through Miniflare D1 SQL, construct the package D1 client through `createD1Client`, execute the final public store query against that binding, and prove an SQL mutation changes the observed package result. A callback/identity wrapper used only because D1 lacks a `BEGIN` API must not replace SQL execution with static JS rows or advertise unproved rollback semantics.
- Postgres must start the repository’s embedded **real** PostgreSQL harness, execute real DDL/DML and the same public store read, and fail closed if startup/fixture execution is absent. Both named fixtures need explicit execution evidence in the same focused conformance run; no skip or optional environment branch.
- Compare normalized ordering, arbitrary legacy content type/status strings, dates, numeric/boolean lanes, JSON/reference metadata, and deep immutability across adapters. Ensure parity assertions cannot pass if one fixture silently returns the other fixture’s data.
- Strengthen malformed-row discrimination where needed: wrong/missing scalar types, invalid dates/numerics/booleans, exactly-one-lane violations, malformed reference lane combinations/JSON, unsupported adapter names, and non-plain or unsafe nested metadata must return the package’s structural contract error rather than leak a raw parser/driver error or be coerced.

### B. Exact transaction capability identity

- Prove same-callback identity succeeds on D1 and Postgres, independently minted valid transactions fail with structural `transaction-capability`/`identity-mismatch` before effects, and an inactive callback capability or plain querier cannot be used as an active transaction.
- Keep the same callback transaction object and opaque identity; do not construct an intersection, cast a querier into a transaction, expose a public identity factory, or persist identity in inputs/results.
- Structural guards must work across duplicate package copies and must not rely on `instanceof`.

### C. Public root exports

- Compare root export inventories to `33e7a1ba`. Preserve every prior runtime export and prior consumable type name while exporting the Task 17 schema records, flat readers, transaction aliases/assertions, contract errors/guards, and definition-version contracts.
- Resolve the legacy/final fields naming collision deliberately: the authoritative `FieldDefinitionVersion['definition']` must be the final bounded `FieldGroup`, while any retained legacy pre-migration store types remain explicit compatibility types and do not widen the immutable definition contract. Root-only compile fixtures must make both intended surfaces unambiguous.
- Keep feed and other capability exports on their existing subpaths; do not accidentally pull them into the core barrel.

### D. Exact immutable `FieldDefinitionVersion` contract

- `FieldDefinitionVersion.definition`, `NormalizedFieldsRegistryRecord.definition`, and Task 7’s future persisted version row must share one authoritative final `FieldGroup`, not `FieldStorageValue`, the legacy group, a union, or a second migration alias.
- Match the complete fields spec: final group metadata, every built-in discriminant/settings shape, recursive group/repeater/flexible/clone definitions, conditional and location rule groups, exact required/optional keys, and prohibited defaults for password/presentation fields.
- Parse from `unknown` into detached, deeply frozen plain data. Inspect property descriptors before reads; never execute getters, `toJSON`, callbacks, expressions, `RegExp`, provider code, or executable imports. Reject accessors, symbols, cycles/shared unsafe graphs, forbidden prototype-pollution keys, non-finite numbers, unknown keys/settings/types, malformed operator/value matrices, duplicate sibling field/layout identities, and group-key mismatch through `FieldDefinitionVersionError` plus its structural guard.
- Enforce the spec’s whole-graph hard defaults, including maximum recursive definition depth 12, maximum **1,000 total materialized field nodes** (not merely 1,000 per child array), bounded collections/strings/rules/paths, and a UTF-8 byte budget before allocation-heavy normalization. Add at-limit and one-over tests for global count, depth, and payload size.
- Validate field-specific defaults and min/max/dimension/choice/date/path constraints rather than accepting an arbitrary serializable `defaultValue` that contradicts its discriminant.
- Preserve Task 7/Task 8 forward compatibility by keeping immutable row/envelope identity stable and parser dispatch separable from recursive traversal. Future explicitly injected immutable `FieldTypeRegistry` descriptors must be addable without changing `FieldDefinitionVersion` or Task 7’s row shape; do not add a module-global registry, accept unknown settings, or implement Task 8’s value engine now.

### E. Scope and standards

- Keep reads of the existing flat corpus non-enforcing: do not coerce unknown content types/statuses or begin recursive writes/migrations.
- Keep package seams host-agnostic, validator-agnostic, web-standard, typed, and contextful. No new `node:` imports in package runtime, provider SDK, framework import, global mutable registry, source-app default, or warning suppression.
- If the canonical spec is genuinely contradictory, stop implementation and amend the canonical spec/plan before divergence; otherwise do not touch docs.

## 5. Focused quality gates

Run commands separately so each exit status is visible. Prefix every command exactly with the required PATH assignment.

```bash
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/fields exec vitest run --config vitest.config.ts src/definition-version.test.ts src/index.test.ts
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/content exec vitest run --config vitest.config.ts src/index.test.ts src/conformance.test.ts
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/fields exec vitest run --config vitest.config.ts src/conformance.test.ts
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/content typecheck
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/fields typecheck
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/content build
PATH=$HOME/.nvm/versions/node/v22.23.1/bin:$PATH pnpm --filter @platform-modules/fields build
```

Also run `git diff --check`. Inspect the complete successful content and fields build output and resolve every owned warning/notice; zero exit alone is insufficient for this warning-clean task. Do not substitute package-wide mocks for either named conformance file.

If any P1/P2 repair changes behavior covered by another focused co-located test, run that test as well with the same PATH. Do not broaden into the monorepo gate unless a changed shared file makes it necessary.

## 6. Independent review loop

1. After the diff is stable and all gates are clean, launch an independent reviewer using the explicitly requested `gpt-5.6-sol` model at `low` thinking. Give it the complete diff `33e7a1ba..HEAD` (including current uncommitted repairs) and all sources of truth listed above.
2. Ask for actionable findings only, labeled P1/P2/P3, and explicitly require review of warning removal (no suppression), genuine D1/real-PG execution, parity, export preservation, structural guards, exact transaction identity, malformed-input discrimination, `FieldDefinitionVersion.definition === FieldGroup`, whole-graph bounds/deep immutability, and Task 7/Task 8 forward compatibility.
3. Wait for the report. Fix every P1/P2. If a P1/P2 causes any code/test/config change, rerun all focused quality gates and repeat the independent review on the new full diff.
4. Completion requires a final reviewer report stating P1/P2 none. Record any P3 only when genuinely non-blocking and outside this fixed scope; do not use P3 to defer a contract defect.

## 7. Scope audit and commit

1. Inspect `git diff --name-only 33e7a1ba..HEAD` and the final uncommitted diff. Reject changes under forbidden paths and revert unrelated edits. Expected new work is limited to the relevant content/fields source/tests/config and this ADW plan artifact; package metadata/lockfile changes require a demonstrated dependency need.
2. Run `git diff --check`, inspect `git status --short`, and ensure no generated `dist`, logs, temporary warning captures, or review scratch files are tracked.
3. Commit all scoped repairs and this ADW artifact with an imperative subject such as `Finalize content-model Task 17`.
4. Verify `git status --porcelain` is empty. Handoff the final commit SHA, per-command exit statuses, confirmation of warning-free build output, both executed adapter identities, and the final independent review’s P1/P2-none result.

## Definition of done

Task 17 is complete only when the work is committed and clean; content and fields final stores execute genuine D1 SQL and embedded real PostgreSQL with equivalent results; transaction identities are exact; malformed data fails structurally; root exports preserve the established surface; `FieldDefinitionVersion.definition` is exactly the deeply immutable, bounded final `FieldGroup` contract and remains Task 7-forward; every focused test/typecheck/build exits zero under Node 22; build output contains no owned warnings; and the final independent `gpt-5.6-sol/low` review reports no P1/P2.