query-orchestration/plans/metadata.progress.review.codex.md

Mutable Metadata Progress Review

Scope: this is a file-state review only. Q reported that task fullsuite:ci is now passing; I did not rerun it. I reviewed the current tracking doc, journal, docs, and mutable-metadata implementation files.

Findings

1. High: the progress artifacts are still contradictory enough to mislead the next handoff. The tracking file marks Milestone 0-INT as COMPLETE at plans/mutable.metadata.plan.combo.v4.tracking.md, but the same section still contains unchecked “What remains” items at plans/mutable.metadata.plan.combo.v4.tracking.md. More importantly, the detailed Milestone 3 checklist is still entirely [ ] at plans/mutable.metadata.plan.combo.v4.tracking.md, even though reset query/service/handler work already exists in internal/customschema/service_reset.go and api/queryAPI/customschemas_reset.go. The journal is also stale: it still says api/queryAPI/customschemas_reset_test.go “does NOT exist yet” at journals/implement_mutableMetadata.md, but that file now exists and contains reset endpoint tests at api/queryAPI/customschemas_reset_test.go. There is also a naming mismatch: the tracking checklist still refers to ResetDocumentMetadataResponse at plans/mutable.metadata.plan.combo.v4.tracking.md, while the actual spec/code use DocumentMetadataResetResponse at serviceAPIs/queryAPI.yaml. The codebase has moved forward; the handoff documents have not.

2. Medium-High: SetDocumentMetadata still does not implement the documented retry-on-unique-violation fallback. The tracking file marks this complete at plans/mutable.metadata.plan.combo.v4.tracking.md, but the implementation in internal/customschema/service_metadata.go computes nextVersion, performs a single CreateDocumentCustomMetadata call, and returns immediately on insert error. I could not find a 23505 / uq_doc_custom_metadata_version branch or retry loop. The test file also has happy-path, validation, and pagination coverage in internal/customschema/service_metadata_test.go, but no injected unique-violation retry case. So this is still both an implementation gap and a tracking-doc overstatement.

3. Medium: Milestone 3 coverage is still materially short of the planned test surface, even if the suite is green. The plan/tracking expect a full upgrade flow, schema-version concurrency, and reset/version authorization-matrix coverage at plans/mutable.metadata.plan.combo.v4.tracking.md. The current reset test file covers only the reset endpoint’s direct status paths: missing subject, not found, legacy conflict, happy path, and idempotent clean-doc in api/queryAPI/customschemas_reset_test.go, api/queryAPI/customschemas_reset_test.go, api/queryAPI/customschemas_reset_test.go, api/queryAPI/customschemas_reset_test.go, and api/queryAPI/customschemas_reset_test.go. I found no current tests for the planned “bind v1 -> write -> create v2 -> reset -> rebind -> write” flow, no end-to-end schema version concurrency test, and no reset/version auth-matrix test in the checked api/queryAPI test files. A green suite here means “no current regressions detected,” not “Milestone 3 is fully proven against its own checklist.”

4. Medium: the human-facing docs still stop at Milestone 2 even though the OpenAPI spec now exposes reset-metadata. The source-of-truth spec contains POST /super-admin/documents/{id}/reset-metadata and DocumentMetadataResetResponse at serviceAPIs/queryAPI.yaml and serviceAPIs/queryAPI.yaml. But the summary doc’s quick-reference table stops at PATCH /super-admin/documents/{id}/schema plus the four /custom-metadata routes at docs/ai.generated/queryapi.summary.md, and the service description section likewise ends with schema assignment at docs/ai.generated/queryapi.summary.md. The API documentation authorization table also lists PATCH /super-admin/documents/{id}/schema and the four /custom-metadata routes, but not reset-metadata, at docs/ai.generated/03-api-documentation.md. This is now a real user-facing drift, not just a future-doc TODO.

5. Medium-Low: the new metadata audit events still emit an empty client_id, which weakens the audit trail. AuditRecord carries ClientID in internal/customschema/audit.go, but SetDocumentMetadata hardcodes ClientID: "" at internal/customschema/service_metadata.go even though it already loaded the bound schema row earlier at internal/customschema/service_metadata.go. ResetDocumentMetadata similarly records metadata.reset without populating ClientID at internal/customschema/service_reset.go. schema.assign does populate client id correctly at internal/customschema/service_metadata.go, so the inconsistency is limited to the newer metadata/reset write paths.

Overall Read

The codebase is in a materially better state than the previous review: reset-metadata now exists in the spec and code, and there is at least direct endpoint coverage for the reset path. The remaining problems are mostly about truthfulness and completeness rather than obvious compilation/runtime breakage: the tracking/journal state is behind the code, the planned retry fallback in SetDocumentMetadata still has not landed, Milestone 3’s full planned test surface is not yet present, and the human-facing docs have not caught up with the new reset endpoint.