[object Object]

← back to NationalPaperHangers Yfr2 C3

harden measure attribution migration proof

b9ca75816703b84d5eb03c75ae1be95dbbe66ee5 · 2026-08-29 14:08:18 -0700 · Steve Abrams

Files touched

Diff

commit b9ca75816703b84d5eb03c75ae1be95dbbe66ee5
Author: Steve Abrams <steve@designerwallcoverings.com>
Date:   Sat Aug 29 14:08:18 2026 -0700

    harden measure attribution migration proof
---
 db/migrations/024_measure_attribution_phase_a.sql |  7 ++---
 lib/domain/measure-tier.js                        | 17 ++++++++++---
 tests/measure-attribution-sql.test.js             |  6 +++++
 tests/measure-tier.test.js                        | 31 +++++++++++++++++++++++
 verification/e2e-proof.json                       | 27 ++++++++++++++------
 5 files changed, 74 insertions(+), 14 deletions(-)

diff --git a/db/migrations/024_measure_attribution_phase_a.sql b/db/migrations/024_measure_attribution_phase_a.sql
index 7e77fcf..1be9485 100644
--- a/db/migrations/024_measure_attribution_phase_a.sql
+++ b/db/migrations/024_measure_attribution_phase_a.sql
@@ -19,7 +19,9 @@ ALTER TABLE measure_jobs
   ADD COLUMN IF NOT EXISTS consent_purposes TEXT[],
   ADD COLUMN IF NOT EXISTS contact_preference TEXT,
   ADD COLUMN IF NOT EXISTS measurement_method TEXT,
-  ADD COLUMN IF NOT EXISTS room_captures JSONB DEFAULT '[]'::jsonb,
+  -- NULL deliberately preserves the distinction between legacy unknown media
+  -- and an explicit empty capture set.
+  ADD COLUMN IF NOT EXISTS room_captures JSONB,
   ADD COLUMN IF NOT EXISTS surface_state TEXT,
   ADD COLUMN IF NOT EXISTS access_constraints TEXT[],
   ADD COLUMN IF NOT EXISTS timeline_band TEXT,
@@ -116,8 +118,7 @@ ALTER TABLE measure_jobs
     REFERENCES measure_job_routing_attempts (id, measure_job_id, installer_id)
     ON DELETE RESTRICT,
   ADD CONSTRAINT measure_jobs_current_route_pair_ck CHECK (
-    (current_routing_attempt_id IS NULL AND routed_to IS NULL)
-    OR (current_routing_attempt_id IS NOT NULL AND routed_to IS NOT NULL)
+    current_routing_attempt_id IS NULL OR routed_to IS NOT NULL
   ) NOT VALID;
 
 ALTER TABLE bookings
diff --git a/lib/domain/measure-tier.js b/lib/domain/measure-tier.js
index d56b1a5..ec4e856 100644
--- a/lib/domain/measure-tier.js
+++ b/lib/domain/measure-tier.js
@@ -14,16 +14,27 @@ function text(value) {
 }
 
 function positive(value) {
-  const number = typeof value === 'number' ? value : Number(value);
+  if (typeof value !== 'number' && !(typeof value === 'string' && /^(?:0|[1-9]\d*)(?:\.\d+)?$/.test(value))) {
+    return false;
+  }
+  const number = Number(value);
   return Number.isFinite(number) && number > 0;
 }
 
 function validDate(value) {
-  if (!value) return false;
+  if (!(value instanceof Date) && !(typeof value === 'string'
+    && /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d{1,6})?(?:Z|[+-]\d{2}:\d{2})$/.test(value))) {
+    return false;
+  }
   const date = value instanceof Date ? value : new Date(value);
   return Number.isFinite(date.getTime());
 }
 
+function positiveIntegerId(value) {
+  if (typeof value === 'number') return Number.isSafeInteger(value) && value > 0;
+  return typeof value === 'string' && /^[1-9]\d*$/.test(value);
+}
+
 function validEmail(value) {
   const normalized = text(value);
   return normalized !== null && EMAIL_RE.test(normalized);
@@ -61,7 +72,7 @@ function classifyMeasureTier(job = {}) {
   const purposes = Array.isArray(job.consent_purposes) ? job.consent_purposes : [];
   const routingConsent = fail(purposes.includes('installer_routing'), 'missing_installer_routing_consent');
   const validZip = fail(ZIP_RE.test(text(job.zip) || ''), 'invalid_or_missing_zip');
-  const routedTo = fail(Number.isSafeInteger(Number(job.routed_to)) && Number(job.routed_to) > 0, 'missing_routed_to');
+  const routedTo = fail(positiveIntegerId(job.routed_to), 'missing_routed_to');
   const routedAt = fail(validDate(job.routed_at), 'missing_routed_at');
   const eligibleState = fail(ELIGIBLE_STATES.has(job.lead_state), 'ineligible_lead_state');
 
diff --git a/tests/measure-attribution-sql.test.js b/tests/measure-attribution-sql.test.js
index 8ac4ec2..225feb3 100644
--- a/tests/measure-attribution-sql.test.js
+++ b/tests/measure-attribution-sql.test.js
@@ -59,3 +59,9 @@ test('known invalid or superseded SQL shapes are absent', () => {
   assert.doesNotMatch(sql, /measure_booking_one_primary_per_booking_idx/i, 'old mutable primary index');
   assert.doesNotMatch(sql, /FOREIGN KEY \(measure_job_id, measure_routed_to\)/i, 'old current-route attribution binding');
 });
+
+test('legacy unknown captures and routed rows remain compatible', () => {
+  assert.doesNotMatch(sql, /room_captures\s+JSONB\s+DEFAULT/i);
+  assert.match(sql, /current_routing_attempt_id IS NULL OR routed_to IS NOT NULL/i);
+  assert.doesNotMatch(sql, /current_routing_attempt_id IS NULL AND routed_to IS NULL/i);
+});
diff --git a/tests/measure-tier.test.js b/tests/measure-tier.test.js
index ee0b798..8cd81d3 100644
--- a/tests/measure-tier.test.js
+++ b/tests/measure-tier.test.js
@@ -162,3 +162,34 @@ test('invalid ZIP, date, method, and preferred-contact formats are ineligible',
     assert.ok(result.reason_codes.includes(reason), field);
   }
 });
+
+test('accepts database numeric strings but rejects JavaScript coercion traps', () => {
+  assert.equal(classifyMeasureTier(base({ sqft: '240', routed_to: '42' })).tier, 'standard');
+  for (const value of [true, false, [], [240], {}, ' 240', '240x', '-1']) {
+    assert.equal(classifyMeasureTier(base({ sqft: value })).tier, 'ineligible', `sqft=${String(value)}`);
+  }
+  for (const value of [true, [], [42], {}, '042', '42.0', '-1']) {
+    assert.equal(classifyMeasureTier(base({ routed_to: value })).tier, 'ineligible', `routed_to=${String(value)}`);
+  }
+});
+
+test('accepts Date/ISO timestamps and rejects coercible non-timestamps', () => {
+  assert.equal(classifyMeasureTier(base({ captured_at: new Date('2026-08-01T12:00:00Z') })).tier, 'standard');
+  for (const value of [true, 1, [], {}, '2026-08-01', '08/01/2026']) {
+    assert.equal(classifyMeasureTier(base({ captured_at: value })).tier, 'ineligible', String(value));
+  }
+});
+
+test('room capture dimensions reject coercible arrays and booleans', () => {
+  const spec = {
+    rolls: 8, wall_count: 4, material: 'grasscloth', timeline_band: 'within_90_days',
+    surface_state: 'painted_drywall'
+  };
+  for (const wall_width_ft of [true, [12], {}, ' 12']) {
+    const result = classifyMeasureTier(base({ ...spec, room_captures: [{
+      wall_width_ft, wall_height_ft: 9,
+      photo_url: '/uploads/bookings/0123456789abcdef01234567.jpg'
+    }] }));
+    assert.equal(result.tier, 'standard', String(wall_width_ft));
+  }
+});
diff --git a/verification/e2e-proof.json b/verification/e2e-proof.json
index f83aca3..055ac26 100644
--- a/verification/e2e-proof.json
+++ b/verification/e2e-proof.json
@@ -1,7 +1,7 @@
 {
   "intent": "Implement and verify the first local-only NPH measure-attribution Phase-A slice: pure tier predicates plus an unapplied additive structural migration.",
   "risk_tier": "R1",
-  "environment": "isolated git worktree; no database connection, runtime service, provider, or customer surface",
+  "environment": "isolated git worktree plus uniquely named disposable local PostgreSQL database; no canonical database, runtime service, provider, or customer surface",
   "build_identity": {
     "branch": "yfr2-c3-nph-attribution",
     "parent_commit": "b672493a7a58cc4e84ddd55e41fca84a13521033",
@@ -12,7 +12,7 @@
   "commands": [
     {
       "command": "node --test tests/measure-tier.test.js tests/measure-attribution-sql.test.js",
-      "result": "PASS: 15 tests, 0 failures"
+      "result": "PASS: 19 tests, 0 failures"
     },
     {
       "command": "node --check lib/domain/measure-tier.js",
@@ -20,7 +20,7 @@
     },
     {
       "command": "node --test tests/measure-kit.test.js tests/measure-tier.test.js tests/measure-attribution-sql.test.js",
-      "result": "PASS: 29 tests, 0 failures"
+      "result": "PASS: 33 tests, 0 failures"
     },
     {
       "command": "npm test",
@@ -33,6 +33,14 @@
     {
       "command": "git diff --check",
       "result": "PASS"
+    },
+    {
+      "command": "apply db/schema.sql, migrations 001-023, seed two legacy rows, then apply migration 024 in nph_yfr2_c3_20260829",
+      "result": "PASS: PostgreSQL parsed and committed the complete fresh migration path; both legacy rows retained room_captures IS NULL"
+    },
+    {
+      "command": "update seeded legacy routed row after migration; exercise current-route pairing and exact-installer FK negatives",
+      "result": "PASS: legacy update succeeded; valid exact route succeeded; missing routed_to and mismatched installer were rejected with the expected constraint classes"
     }
   ],
   "assertions": [
@@ -44,12 +52,12 @@
     {
       "boundary": "migration-contract",
       "verdict": "PASS",
-      "evidence": "Static verifier proves additive transaction shape, routing root/successor rules, exact booking/attempt installer FKs, append-only candidate/decision/current model, provider-scoped event idempotency, and absence of known invalid partial-UNIQUE and superseded mutable-primary shapes."
+      "evidence": "Static verifier proves the intended shape, and PostgreSQL executed the complete fresh schema/migration chain through 024. Seeded legacy captured/routed rows retained unknown captures as NULL, remained update-compatible, and exact-route positive/negative constraints behaved as designed."
     },
     {
       "boundary": "database-side-effect",
       "verdict": "PASS",
-      "evidence": "Migration was read only as a file and never passed to psql or any database client. No DB query or apply occurred."
+      "evidence": "Migration was applied only to uniquely named disposable local database nph_yfr2_c3_20260829. Canonical national_paper_hangers remained read-only and was queried only for aggregate/schema types; no canonical write occurred."
     },
     {
       "boundary": "runtime-and-external-side-effect",
@@ -59,7 +67,7 @@
     {
       "boundary": "broader-existing-suite",
       "verdict": "SKIP",
-      "reason": "Three existing suites require supertest, which is not installed in the isolated worktree. Network dependency installation was outside this task's external-call gate. Existing dependency-free measure-kit tests passed 14/14. This does not skip the changed critical path, which is covered by 15/15 new tests."
+      "reason": "Three existing suites require supertest, which is not installed in the isolated worktree. Network dependency installation was outside this task's external-call gate. Existing dependency-free measure-kit tests passed 14/14. The changed predicate and migration paths are covered by 19/19 new tests plus the disposable PostgreSQL rehearsal."
     }
   ],
   "negative_checks": [
@@ -67,10 +75,13 @@
     "missing consent/contact/routing/scope cannot be inferred",
     "product_name and notes cannot satisfy specification fields",
     "malformed room captures cannot produce spec_ready",
+    "boolean, array, padded, malformed, and other coercible numeric/date values cannot qualify",
+    "legacy routed records remain update-compatible and legacy room captures remain NULL",
+    "current route without routed_to and installer-mismatched exact routes are rejected by PostgreSQL",
     "inline partial UNIQUE and mutable touch_kind primary shapes are rejected",
     "migration text contains no INSERT, UPDATE, DELETE, TRUNCATE, destructive DROP, trigger, or feature activation"
   ],
-  "cleanup": "No database fixtures or runtime state were created. Failed offline npm installation left no node_modules directory. Source changes are retained in the isolated worktree for review and commit.",
+  "cleanup": "Disposable database nph_yfr2_c3_20260829 was dropped after assertions. Failed offline npm installation left no node_modules directory. Source changes are retained in the isolated worktree for review and commit.",
   "overall_verdict": "PASS_WITH_ENVIRONMENTAL_SUITE_LIMIT",
-  "residual_risk": "SQL integrity is statically verified but not parsed/executed against a disposable PostgreSQL instance in this bounded slice. Full dependency-backed existing tests should be rerun in an environment with the locked dependencies available before integration."
+  "residual_risk": "The full dependency-backed existing HTTP suite remains unavailable because locked dependencies are not installed. Migration rehearsal proves a fresh full-chain apply and representative legacy/constraint behavior, but production apply, feature activation, API wiring, and backfill remain explicitly out of scope and gated."
 }

← 42fc3d7 add local measure attribution phase a  ·  back to NationalPaperHangers Yfr2 C3  ·  reject impossible measure timestamps f3845d6 →