Status Current

Path substrate/cleanup/phase-61-implementation-notes-v0_1.html

←Back

Loomworks · Phase 61 · Implementation Notes

Phase 61 Implementation Notes — v0.1

Version. 0.1

Date. 2026-05-24

Author. Claude Code

Engine HEAD. 616bab9 (merge of phase-61-audit-cleanup)

Tag. phase-61-audit-cleanup

Companion documents.

phase-61-cr-audit-cleanup-v0_1.md — the CR these notes close.
loomworks-cr-a-audit-replacement-design-v0_1.md — the design these notes execute.
loomworks-substrate-hygiene-scoping-note-v0_2.md — the scoping that authorized the three-sibling-CR shape.
loomworks-foray-integration-locations-v0_1.md — Step 1 inventory.

1. What landed

Phase 61 closed in three commits on the engine, merged as 616bab9:

5080ff9 — Create src/loomworks/foray/ module + Alembic migration 0071_audit_events_replacement
f0934da — Audit module rename + reserved-emit at three call sites
3facbbd — Update tests for audit.events rename

The cleanup work proceeds as planned by the scoping note v0.2: CR-A (this CR) shipped first, smallest, validates the reserved-location pattern; CR-B (Phase 25 wiring) and CR-C (credit substrate) ship next.

Substrate result

audit.foray_events no longer exists. audit.events carries 37 historical rows (preserved 1:1 across the rename). Schema:

Column	Type	Nullable	Notes
`id`	UUID	NOT NULL (PK)	`gen_random_uuid()` default
`event_kind`	VARCHAR(64)	NOT NULL	Today: `"setting_change"` only
`actor_person_id`	UUID	NOT NULL	No DB-level FK (see §3)
`engagement_id`	UUID	NULL	No DB-level FK (see §3)
`payload`	JSONB	NOT NULL	Type-specific data
`rationale`	TEXT	NULL	New; NULL today; populated by future writes
`occurred_at`	TIMESTAMPTZ	NOT NULL	Renamed from `timestamp`

Indexes: actor_ts, kind_ts (down from four; tx_id index gone with the column; engagement_ts dropped — no query exercised it).

Dropped from original 0068 schema: tx_id (FORAY transaction concept; always 1:1 with id; not referenced); signature LargeBinary (FORAY-readiness; never written).

Module result

src/loomworks/audit/ carries the renamed AuditEventRow class + EVENT_KIND_SETTING_CHANGE constant. Module docstrings drop FORAY framing.

src/loomworks/foray/ is new — two files, ~50 lines total. Holds _foray_reserved_emit(event_kind, payload) as a no-op debug log. CR-B will move _ANCHOR_PRIORITY into this module; CR-C will extend with value-flow emit support; future FORAY integration ship adds the real ForayClient wrapper.

Call-site result

Three call sites carry # FORAY_RESERVED_LOCATION: audit.setting_change comment markers + _foray_reserved_emit("audit.setting_change", {...}) invocations:

src/loomworks/orchestration/tune_setting.py:498 (Companion-said enum-set, message_order)
src/loomworks/orchestration/tune_setting.py:610 (Companion-said numeric, blur_intensity / silence_submit_seconds)
src/loomworks/api/routers/me_settings.py:133 (button-click PUT /me/settings/{key})

Line numbers shifted slightly from the design's 489 / 596 / 117 because the reserved-emit blocks add 5 lines each above the return.

Test result

Metric	Baseline	Final
Collected	2708	2709 (+1)
Passed	(n/a — see §2)	2674
Skipped	33	33
Failed	2 (pre-existing)	2 (same pre-existing)

The +1 is test_foray_reserved_emit_is_importable_and_callable — a single import-level smoke test for the no-op emitter.

Four test files updated:

tests/test_voice_tune_setting_audit.py (renamed from _foray_audit; SQL/ORM/constant rewrites; function renames)
tests/test_voice_silence_submit_setting.py (line-437 SQL)
tests/test_voice_person_settings.py (line-283 SQL + function rename)
tests/conftest.py (TRUNCATE chain)

2. Pre-existing failures (not introduced by CR-A)

Both confirmed against main baseline before any CR-A changes:

tests/test_api_assertion_retract.py::test_api_assertion_retract — TypeError: Object of type ValueError is not JSON serializable at src/loomworks/api/app.py:461. The validation-error logging handler builds a _JSONResponse containing a ValueError object; FastAPI tries to JSON-encode it and fails. This is a pre-existing bug in the error-handling path of _log_request_validation. Has nothing to do with audit substrate.
tests/test_phase_44_evaluator.py::test_concern_scan_does_not_re_fire_within_dedup_window — assert 1 == 0 on concern_based_created count. Dedup-window flake; concern not deduplicated when re-fired within the dedup window. Pre-existing evaluator behavior; unrelated to audit.

Filed as observations only; out of CR-A scope.

3. Design D5 deviation — FKs dropped (reserved slot 1 consumed)

The single substantive deviation from the design and CR.

What the design said (D5): add foreign keys actor_person_id -> persons(id) RESTRICT and engagement_id -> engagements(id) RESTRICT. Rationale: zero practical cost (Loomworks never deletes persons or engagements), protection against orphan-audit rows through bugs.

What surfaced during execution:

ORM-level FK problem. Declaring ForeignKey("persons.id") on the audit Mapped[...] columns caused SQLAlchemy to resolve the referenced table through the audit-only DeclarativeBase metadata at flush time → NoReferencedTableError: Foreign key associated with column 'events.engagement_id' could not find table 'engagements' with which to generate a foreign key to target column 'id'. This entanglement breaks the documented "independent metadata" pattern that the audit module shares with loomworks.credit.models and loomworks.system_config.models.
DB-level FK problem. Removing the ORM-level ForeignKey() declaration but keeping the FK constraint in the migration (DB enforces; ORM doesn't reference) surfaced a second issue: the audit tests use fabricated uuid4() engagement_ids that don't correspond to real engagement rows. The DB-level FK rejected those inserts → ForeignKeyViolationError: insert or update on table "events" violates foreign key constraint "events_engagement_id_fkey".
Cost-benefit reckoning. The orphan-row protection FKs were meant to provide guards a class of bug that cannot occur in Loomworks (engagements and persons are never deleted; the original audit.foray_events from migration 0068 never had FKs either, and the substrate worked correctly in production for the FORAY-audit-for-settings ship's lifetime). The test-pattern conflict is real and meaningful; reworking three audit tests to create real engagements before each call would be a meaningful refactor of test setup with no production benefit.

Decision: drop FKs from both the ORM model and the migration. The audit substrate now matches the original 0068 posture exactly minus the FORAY framing. Trade-off captured in:

Migration docstring at migrations/versions/0071_audit_events_replacement.py:33-41
AuditEventRow class docstring at src/loomworks/audit/models.py:32-40

Future CR-B / CR-C can re-introduce FKs if a concrete use case emerges; today they did not earn their keep.

Reserved slot accounting: 1 of 2 consumed by this deviation.

4. Decision retrospective — D9 per-call-site reserved emit

Held up cleanly during execution. Three near-identical 4-line emit blocks at three call sites was not awkward, did not feel like ceremony, and made the business-event-vs-persistence-write distinction visible in the code. The audit write goes through one helper (write_setting_change_event); the reserved emit fires at each business event.

CR-B (memory-events injection point) and CR-C (12 constructor sites in credit) should inherit the per-call-site pattern without revisiting. If CR-C's 12 sites feels like ceremony where 3 did not, that's a methodology surface for the next scoping pass.

5. What CR-B and CR-C inherit

From the substrate side

src/loomworks/foray/ exists. CR-B can add the relocated _ANCHOR_PRIORITY registry to a new file (e.g. src/loomworks/foray/anchor_priority.py) without restructuring the module. CR-C can add a value-flow emit helper alongside reserved_emit.py.
Migration pattern validated. Atomic create-table + data-copy + drop-old-table in a single Alembic file with reversible downgrade. CR-C will follow the same pattern at larger scale (12 constructor sites; PostgreSQL trigger replacement).
No-FK posture. The substrate-cleanup CRs should not add FKs unless a concrete use case emerges. The pattern set by CR-A: keep FK-free, preserve independent metadata.

From the call-site pattern

Comment marker: # FORAY_RESERVED_LOCATION: <namespace>.<event_kind> immediately above the call.
Namespaced emit kind: "audit.<...>", "memory.<...>", "credit.<...>" — substrate-prefixed so future FORAY integration can route by substrate.
Payload shape: {"setting_key": ..., "action": ...} for setting changes. CR-B's payload shape for memory events depends on what the FORAY integration would actually attest — leave that for the actual integration ship rather than guessing now.

From the testing pattern

One smoke test per CR for the foray module. CR-A added a single test_foray_reserved_emit_is_importable_and_callable. CR-B and CR-C don't need to re-add the same test; they can extend tests/test_foray_reserved_emit.py or similar if richer assertions become useful.
Cascade tests catch reserved-emit regressions transitively. The three call sites' existing voice-tune-setting cascade tests cover the emit path (no test failures means the emit was reached; the function being no-op means there's nothing else to assert).

From the documentation side

CR header baseline numbers should be re-verified at CR-execution time. This CR's header claimed 2673 pytest from MEMORY.md; actual baseline at engine 2f04e7a was 2708. The CR's pre-flight Step 0 should re-collect the baseline; the CR's acceptance criteria should use the re-collected number rather than the header's claim.

6. Future ledger items

Items surfaced during CR-A execution that don't fit CR-A but should be tracked:

The test_api_assertion_retract JSON-encoding bug is a real pre-existing defect, not just a CR-A non-finding. The _log_request_validation handler at app.py:461 builds a JSONResponse containing a ValueError object; FastAPI's renderer raises TypeError. Worth a separate small ship to fix (probably: str(exc) or repr(exc) inside the response body).
The test_phase_44_evaluator dedup-window flake is harder to characterize without deeper investigation; it may be a real ordering / timing issue in concern dedup or it may be a fixture-state issue. Worth investigation when CR-C (credit substrate) is on the agenda since both touch evaluator surfaces.
The test DB engagement_display_identifier_seq overflow known follow-on was workaround-applied during CR-A execution (ALTER SEQUENCE ... RESTART WITH 1). Conftest still doesn't reset the sequence between runs; the existing follow-on stays open.

Items NOT for the ledger (resolved within CR-A):

The FK design deviation is closed by this CR. CR-B / CR-C do not re-litigate.
The substrate hygiene scoping note v0.2 needs no v0.3 from CR-A execution. CR-B and CR-C may surface corrections.

7. Confidence

CR-A landed clean. One documented deviation surfaced and was resolved within the reserved-slot budget. The pattern that lands here is the pattern CR-B and CR-C inherit; CR-A validated it at the smallest possible surface as the scoping note v0.2 anticipated.