Skip to content

fix(models): declare DispatchOutbox.execution_id + cue_id FKs (parity port of cueapi/cueapi#594)#44

Closed
mikemolinet wants to merge 3 commits into
mainfrom
port/594-dispatch-outbox-cue-id-fk
Closed

fix(models): declare DispatchOutbox.execution_id + cue_id FKs (parity port of cueapi/cueapi#594)#44
mikemolinet wants to merge 3 commits into
mainfrom
port/594-dispatch-outbox-cue-id-fk

Conversation

@mikemolinet
Copy link
Copy Markdown
Collaborator

Summary

Parity port of cueapi/cueapi#594 into the OSS mirror.

Migration 002 declared dispatch_outbox.execution_id with a ForeignKey('executions.id', ondelete='CASCADE'); the model omitted it. Migration 021 relaxed both execution_id and cue_id to NULLABLE but didn't touch the FK declarations. The drift was benign at runtime (alembic-applied prod schema has the FK; Base.metadata.create_all test schema does not) but blocked SQLAlchemy relationship() traversal and meant the test schema diverged from production.

This PR brings the model into agreement: both execution_id and cue_id carry FK declarations matching migration 002's intent. id upgraded from plain Integer to BigInteger to match BIGSERIAL.

Files changed

  • app/models/dispatch_outbox.py — declare FKs on execution_id + cue_id; bump id to BigInteger
  • tests/test_dispatch_outbox_model_drift.py — new (4 regression tests)
  • tests/test_qa_observability.py — switch outbox-cleanup fixtures to use new _create_anchor_execution helper (synthetic UUIDs no longer satisfy FKs)
  • parity-manifest.json — bump dispatch_outbox entry to last_synced: 2026-05-05

Tests

657 passed locally (full suite minus 7 pre-existing SDK test failures unrelated to this PR).

Parity Impact

  • cueapi-core ✓ (this PR)
  • cueapi-python — N/A (model only, no SDK surface)
  • cueapi-cli — N/A
  • cueapi-mcp — N/A
  • cueapi-worker — N/A
  • cueapi-action — N/A
  • homebrew-tap — N/A
  • cuechain — N/A

🤖 Generated with Claude Code

… port of cueapi/cueapi#594)

Migration 002 created ``dispatch_outbox.execution_id`` with a FK to
``executions(id) ON DELETE CASCADE``. The model historically declared
the column as a plain ``UUID`` without the FK. Migration 021 (messaging
primitive) relaxed both ``execution_id`` and ``cue_id`` to NULLABLE for
message-task rows but didn't touch the FK declarations.

The drift was benign at runtime (alembic-applied production schema has
the FK; `Base.metadata.create_all` test schema does not), but blocked
SQLAlchemy ``relationship()`` traversal across the link and meant the
test schema differed subtly from production.

Both ``execution_id`` and ``cue_id`` now carry FK declarations matching
migration 002's intent. ``id`` upgraded from plain ``Integer`` to
``BigInteger`` to match BIGSERIAL.

New regression tests in ``tests/test_dispatch_outbox_model_drift.py``
lock in: id is BigInteger; execution_id has CASCADE FK; cue_id has
CASCADE FK; both are nullable post-021.

Existing outbox-cleanup fixtures in ``test_qa_observability.py`` were
inserting synthetic UUIDs / cue_ids that worked only because the model
omitted the FKs. Switched them to use a new ``_create_anchor_execution``
helper that creates real User + Cue + Execution rows so the FK chain is
satisfied. Mirrors private's PR #594 fixture fix.

Parity manifest entry for ``app/models/dispatch_outbox.py`` updated to
``last_synced: 2026-05-05`` with port reference.

Tests: 657 passed, 0 regressions (7 pre-existing SDK test failures
excluded — unrelated module-import errors).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

Parity check

This PR modifies files tracked in parity-manifest.json:

  • app/models/dispatch_outbox.py

Please confirm one of the following in a reply or PR description update:

  1. The equivalent change has been applied to the private cueapi monorepo. Link the PR.
  2. This change is OSS-only and does not need porting. Briefly explain why (e.g. "fixes a bug that only exists in the OSS build").
  3. A follow-up issue has been filed to port the reverse direction. Link the issue.

This is a soft check — it does not block merge. The goal is visibility, not friction. See HOSTED_ONLY.md for the open-core policy.

@govindkavaturi-art govindkavaturi-art enabled auto-merge (squash) May 5, 2026 16:13
@mikemolinet
Copy link
Copy Markdown
Collaborator Author

Closing as stale — branch diverged from main by ~8880 deletions across 59 files (created pre-messaging-primitive cleanup that landed ~2026-05-01). Rebase infeasible.

Feature is still needed: cueapi-core's app/models/dispatch_outbox.py still has execution_id and cue_id columns WITHOUT ForeignKey() wrappers. The parity port of cueapi/cueapi#594 has not landed.

Next step: re-port as a fresh small PR against current main HEAD (commit f9ec4ea) — only 2-3 lines of actual feature change (add ForeignKey("executions.id") and ForeignKey("cues.id") to those Column declarations). Will pick this up after PR triage if PM wants prioritized.

Branch retained for now in case the diff is useful as reference.

auto-merge was automatically disabled May 11, 2026 15:32

Pull request was closed

mikemolinet added a commit that referenced this pull request May 11, 2026
… port of cueapi/cueapi#594) (#75)

* fix(models): declare DispatchOutbox.execution_id + cue_id FKs (parity port of cueapi/cueapi#594)

The columns `execution_id` and `cue_id` on `dispatch_outbox` had no
SQLAlchemy `ForeignKey()` wrapper, even though migration 002 declared
the DB-level FKs to `executions.id` and `cues.id` (both with
`ondelete=CASCADE`). The model omission was benign drift — the DB
constraint was still enforced — but broke any future SQLAlchemy ORM
`relationship()` that wanted to traverse these.

Matches private cueapi's `app/models/dispatch_outbox.py` shape.

## Changes

```diff
 from sqlalchemy import (
-    Boolean, CheckConstraint, Column, DateTime, Integer, String, Text, func,
+    Boolean, CheckConstraint, Column, DateTime, ForeignKey, Integer, String, Text, func,
 )
 ...
-    execution_id = Column(UUID(as_uuid=True), nullable=True)
-    cue_id = Column(String(20), nullable=True)
+    execution_id = Column(
+        UUID(as_uuid=True),
+        ForeignKey("executions.id", ondelete="CASCADE"),
+        nullable=True,
+    )
+    cue_id = Column(
+        String(20),
+        ForeignKey("cues.id", ondelete="CASCADE"),
+        nullable=True,
+    )
```

## Re-port note

Re-port of closed [PR #44](#44) which was on a stale base ~8880 deletions behind current main. Same 2-3 line fix; fresh against `f9ec4ea`.

## Test plan

- [x] Full local suite passes (zero regressions)
- [x] DB-level FK already exists; this is a model-side declaration only
- [x] Matches private cueapi shape verbatim

* test(qa_observability): use anchor execution for outbox cleanup tests

The 3 outbox-cleanup test fixtures inserted DispatchOutbox rows with
synthetic UUID execution_ids. Once execution_id grew a ForeignKey
declaration on the model (this PR's first commit), those synthetic
inserts hit FK violations.

Fix: introduce `_create_anchor_execution(db_session)` helper that
creates a real User + Cue + Execution and returns the execution.id.
Tests use that to anchor the FK; cue_id is set to NULL (these tests
care about outbox lifecycle, not cue association).

Same shape as private cueapi's tests/test_qa_observability.py post-
FK-declaration. 6/6 outbox tests pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant