DIG-594: Code Review Findings (2026-11-01)

What was validated, not just read

• The delivery documents were read whole before the code: the branch design v8, the mart design v3, the test plan v9 and the implementation guide v8.
• Data, not only test results: in DEV__DV__CLAUDE_WS, organization_h holds NFL, PMC and the two ghost rows; the facts hold 13 client roles, 17 agent pairs and 10 versions of terms; records_not_accepted reads 3 and rows_missing_key 6; insured_dim reads 1952 and FL as DEV_CLAUDE_WS_ROLE and the values as sent as SYSADMIN.
• The build: the full development build of 2026-11-01 04:43 UTC, 271 of 271 succeeded, no warnings. The red runs, after the merge: 11 planted defects each turned their unit tests red (QA record , section 6.1).
• Naming: the 13 CONFORMED_DM objects are singular things with a class suffix and no source; the satellites name their carrier as the vault’s other satellites do; staging follows this repository’s stg__
  .

  Findings

  F-1 (high): the renamed metrics break Mary’s surfaces. The mart design renamed or retired five release 1 metrics (face_amount_total, policies, policies_written, agents_writing, findings) and the entity client_policy. Mary’s release page tells her to ask for face_amount_total and for policies_held by client_policy__role, and the Ask page and QA retest carry the same names. After promotion she would be told to ask for metrics that no longer exist. Needed: the release page, the Ask page suggestions and the QA retest moved to the new names on their own branch, before QA’s consumer check (Ray, answer 1a).

  F-2 (closed 2026-11-01 07:40 UTC): the two saved queries resolve. q09_client_policies and q10_coverage_by_carrier group by Dimension(‘carrier__carrier_name’), two joins away through the policy. Compiled with MetricFlow 0.213.0, the connector’s own version, against the dev manifest, all six saved queries and the release page’s four prompts compile, and face_amount_current by carrier__carrier_name answers Northfield Life 125,000.00: MetricFlow finds the path itself. The same check found F-8.

  F-3 (medium): the full build did not run on freshly cloned data. The standard proves every change by a full build on a fresh clone; the developer landing clone was not refreshed first, and its vault was built over several sessions. The QA build on fresh clones closes this.

  F-4 (medium): Q-13’s answer shows a SYSTEM row. The one sale with no client number joins to the unknown client, whose source is SYSTEM and whose first-known day is empty, so the arrivals answer has a row Mary will not understand. Needed: the release answers explain it in her words.

  F-5 (medium): the insured mask depends on the reader’s role. insured_dim shows real birth dates and addresses to any session holding SYSADMIN, and CLAUDE_WS_ROLE holds it. Needed: QA’s operational check (QA standard 4.9) shows that Mary’s connector role does not hold SYSADMIN and reads the mask.

  F-6 (low): the key-declaring hook errors in runs of unit tests alone. Builds and the run-operation declare the keys; only a test run with nothing built errors. Not a data risk.

  F-7 (low): the red runs came after the merge. Recorded in the QA record; the Code Review Standard asks for them in the pull request before review.

  F-8 (medium): Q-11 by policy number is not Q-11 by policy. NF-100005 arrived from Northfield Life with its carrier code and from one Sunbelt Senior sale without one, so the vault keeps two policy records under one number, as designed until release 2’s business vault. Grouped by policy number, agents_named answers 1 agent on 14 policy numbers and 2 on 1; the release page’s card, counting records, says 15 and 1. Needed: the page’s prompt and card agree, in Mary’s words.

  Seven-layer coverage, by new model

  Model	L1	L2	L3	L4	L6 and L7
  Organization staging (2) and sources (2)	2	2	2 (OS1)	0	0
  organization_h and its satellites (2)	10	0	10 (OH1 to OH4, OT1 to OT3 twice)	0	0
  The five dimensions	5	0	5 (DG1 to DG3, DL1, DL2)	0	0
  The four role views	4	0	4 (DS1 to DS4)	2 (SM1, SM2)	0
  The four facts	18	0	3 (FV1, FV2, D-47)	1 (FM1)	18 (SR1 to SR9, SQ1 to SQ6, SD1 to SD3)

  Layers 2 and 5 hold nothing new for the mart, as the test plan sets: its views read tested vault tables, and drift is detected at landing.

  What is good

  Ghost members kept once per hub and in every role view, proven by tests that fail without them; the insured mask proven in both roles; every metric, semantic model and column described for Mary’s Claude, and checked by tests; the MART drop script refuses an unknown database before it changes anything; the facts reconcile with the vault by two routes.

  Final state, 2026-11-01

  Finding	State	Evidence
  F-1, the renamed metrics on Mary’s surfaces	Fixed	Pull request 23: prompts as her questions, labels in the Ask tags; QA retest 34 of 34
  F-2, saved queries two joins away	Closed	MetricFlow compiled all six in dev and on QA’s own manifest
  F-3, no build on freshly cloned data	Closed	The dev rehearsal and QA runs 2 to 6, each from fresh clones
  F-4, Q-13’s SYSTEM row	Recorded	Explained in Mary’s words in this release’s release answers
  F-5, the insured mask and the reader’s role	Fixed	Pull request 27; the connector’s session reads 1952 and FL in QA run 6
  F-6, the key hook in unit-test-only runs	Recorded	No data effect; builds declare the keys
  F-7, red runs after the merge	Recorded	Eleven planted defects, each red, in the QA record, section 6.1
  F-8, Q-11 by policy number	Fixed	Pull request 23: 14 and 1, NF-100005 explained; QA retest