Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions backend/app/models/delivery.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"""Single-table project, task, execution, file, and delivery nodes."""

import secrets
from datetime import datetime

from sqlalchemy import (
JSON,
Expand All @@ -15,7 +16,10 @@
Integer,
String,
Text,
event,
or_,
)
from sqlalchemy.engine import Connection
from sqlalchemy.sql import func

from app.db.base import Base
Expand Down Expand Up @@ -179,3 +183,87 @@ class Delivery(LoopNode):

class DeliveryAsset(LoopNode):
__mapper_args__ = {"polymorphic_identity": "delivery_asset"}


_MYSQL_UNSET_DATETIME = datetime(1970, 1, 1, 0, 0, 1)
_MYSQL_NON_NULL_DEFAULTS: dict[str, object] = {
"cloud_project_id": "",
"parent_id": "",
"loop_item_id": "",
"delivery_id": "",
"public_id": "",
"project_key": "",
"name": "",
"title": "",
"storage_prefix": "",
"sequence_number": 0,
"created_by_user_id": 0,
"updated_by_user_id": 0,
"assignee_user_id": 0,
"user_id": 0,
"added_by_user_id": 0,
"source": "",
"status": "",
"priority": "",
"due_at": _MYSQL_UNSET_DATETIME,
"current_delivery_id": "",
"local_project_id": 0,
"device_id": "",
"is_default": False,
"task_user_id": 0,
"task_id": "",
"task_title": "",
"backend_task_id": 0,
"linked_by_user_id": 0,
"linked_at": _MYSQL_UNSET_DATETIME,
"unlinked_at": _MYSQL_UNSET_DATETIME,
"path": "",
"kind": "",
"display_name": "",
"relative_path": "",
"object_key": "",
"content_type": "",
"size_bytes": 0,
"sha256": "",
"source_task_binding_id": "",
"source_task_snapshot": {},
"markdown_object_key": "",
"chat_object_key": "",
"manifest_object_key": "",
"metadata_json": {},
"completed_at": _MYSQL_UNSET_DATETIME,
"delivered_at": _MYSQL_UNSET_DATETIME,
}


def adapt_loop_node_values_for_dialect(
values: dict[str, object], dialect_name: str
) -> dict[str, object]:
"""Convert explicit nulls to sentinels required by the production schema."""
if dialect_name != "mysql":
return values
adapted = values.copy()
for attribute, default in _MYSQL_NON_NULL_DEFAULTS.items():
if attribute in adapted and adapted[attribute] is None:
adapted[attribute] = (
default.copy() if isinstance(default, dict) else default
)
Comment on lines +189 to +250

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Preserve SQL NULL for relationship fields instead of synthetic sentinels. Empty strings and zero IDs are not nullable foreign keys: top-level TODO insertion and parent removal can fail FK validation.

  • backend/app/models/delivery.py#L189-L250: remove foreign-key/relationship fields such as parent_id, loop_item_id, delivery_id, local_project_id, and backend_task_id from MySQL sentinel substitution; migrate the physical schema to allow NULL where the model declares it nullable.
  • backend/app/services/loop_items/service.py#L290-L292: retain this adaptation call only after the helper no longer rewrites parent_id=None.
  • backend/tests/schemas/test_delivery.py#L45-L56: assert that nullable relationship IDs remain None; keep sentinel assertions limited to non-relational scalar fields such as unset datetimes.

As per coding guidelines, persistent backend model changes require an Alembic migration with upgrade and rollback verification.

📍 Affects 3 files
  • backend/app/models/delivery.py#L189-L250 (this comment)
  • backend/app/services/loop_items/service.py#L290-L292
  • backend/tests/schemas/test_delivery.py#L45-L56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/models/delivery.py` around lines 189 - 250, Update
_MYSQL_NON_NULL_DEFAULTS and adapt_loop_node_values_for_dialect so nullable
relationship fields, including parent_id, loop_item_id, delivery_id,
local_project_id, and backend_task_id, preserve SQL NULL instead of receiving
synthetic sentinels; add the required Alembic schema migration with upgrade and
rollback verification. In backend/app/services/loop_items/service.py:290-292,
retain the adaptation call unchanged because it is corrected by the helper fix.
In backend/tests/schemas/test_delivery.py:45-56, assert nullable relationship
IDs remain None and limit sentinel checks to non-relational scalar fields such
as unset datetimes.

Source: Coding guidelines

return adapted


def loop_datetime_is_unset(column: object) -> object:
"""Match unset datetimes in both nullable and sentinel schemas."""
return or_(column.is_(None), column == _MYSQL_UNSET_DATETIME)


@event.listens_for(LoopNode, "before_insert", propagate=True)
def _populate_mysql_non_null_defaults(
_mapper: object, connection: Connection, target: LoopNode
) -> None:
"""Adapt nullable model values to the production MySQL sentinel schema."""
values = {
attribute: getattr(target, attribute) for attribute in _MYSQL_NON_NULL_DEFAULTS
}
adapted = adapt_loop_node_values_for_dialect(values, connection.dialect.name)
for attribute, value in adapted.items():
setattr(target, attribute, value)
7 changes: 6 additions & 1 deletion backend/app/schemas/cloud_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from datetime import datetime
from typing import Literal

from pydantic import BaseModel, ConfigDict, Field
from pydantic import BaseModel, ConfigDict, Field, field_validator

from app.schemas.cloud_project import SnowflakeId

Expand Down Expand Up @@ -40,6 +40,11 @@ class CloudFileResponse(BaseModel):
created_at: datetime
updated_at: datetime

@field_validator("content_type", "sha256", mode="before")
@classmethod
def normalize_empty_optional_text(cls, value: object) -> object:
return None if value == "" else value


class CloudFileListResponse(BaseModel):
items: list[CloudFileResponse]
Expand Down
5 changes: 5 additions & 0 deletions backend/app/schemas/cloud_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ class LocalBindingResponse(BaseModel):
created_at: datetime
updated_at: datetime

@field_validator("device_id", mode="before")
@classmethod
def normalize_empty_device_id(cls, value: object) -> object:
return None if value == "" else value


class CloudProjectMemberCreate(BaseModel):
user_id: int = Field(ge=1)
Expand Down
61 changes: 60 additions & 1 deletion backend/app/schemas/delivery.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from datetime import datetime
from typing import Any, Literal

from pydantic import BaseModel, ConfigDict, Field
from pydantic import BaseModel, ConfigDict, Field, field_validator

from app.schemas.cloud_project import CloudProjectResponse, SnowflakeId

Expand Down Expand Up @@ -58,6 +58,25 @@ class LoopItemResponse(BaseModel):
updated_at: datetime
completed_at: datetime | None

@field_validator("parent_id", "current_delivery_id", mode="before")
@classmethod
def normalize_empty_id(cls, value: object) -> object:
return None if value == "" else value

@field_validator("assignee_user_id", mode="before")
@classmethod
def normalize_empty_user_id(cls, value: object) -> object:
return None if value == 0 else value

@field_validator("due_at", "completed_at", mode="before")
@classmethod
def normalize_unset_datetime(cls, value: object) -> object:
if isinstance(value, datetime) and value == datetime(1970, 1, 1, 0, 0, 1):
return None
if isinstance(value, str) and value.startswith("1970-01-01 00:00:01"):
return None
return value


class LoopItemListResponse(BaseModel):
items: list[LoopItemResponse]
Expand All @@ -75,6 +94,11 @@ class LoopItemAttachmentResponse(BaseModel):
created_by_user_id: int
created_at: datetime

@field_validator("content_type", mode="before")
@classmethod
def normalize_empty_content_type(cls, value: object) -> object:
return None if value == "" else value


class LoopItemAttachmentAccessResponse(BaseModel):
url: str
Expand Down Expand Up @@ -130,6 +154,21 @@ class LoopItemTaskBindingResponse(BaseModel):
linked_at: datetime
unlinked_at: datetime | None

@field_validator("loop_item_id", "task_title", mode="before")
@classmethod
def normalize_empty_text(cls, value: object) -> object:
return None if value == "" else value

@field_validator("backend_task_id", mode="before")
@classmethod
def normalize_empty_task_id(cls, value: object) -> object:
return None if value == 0 else value

@field_validator("unlinked_at", mode="before")
@classmethod
def normalize_unlinked_at(cls, value: object) -> object:
return LoopItemResponse.normalize_unset_datetime(value)


class CloudTaskContextResponse(LoopItemTaskBindingResponse):
project: CloudProjectResponse
Expand All @@ -153,6 +192,11 @@ class DeliveryAssetResponse(BaseModel):
size_bytes: int
sha256: str

@field_validator("content_type", mode="before")
@classmethod
def normalize_empty_content_type(cls, value: object) -> object:
return None if value == "" else value


class DeliveryAssetAccessResponse(BaseModel):
url: str
Expand All @@ -172,6 +216,21 @@ class DeliveryResponse(BaseModel):
delivered_at: datetime | None
assets: list[DeliveryAssetResponse] = Field(default_factory=list)

@field_validator("source_task_binding_id", mode="before")
@classmethod
def normalize_empty_binding_id(cls, value: object) -> object:
return None if value in ("", 0) else value

@field_validator("source_task_snapshot", mode="before")
@classmethod
def normalize_empty_snapshot(cls, value: object) -> object:
return None if value == {} else value

@field_validator("delivered_at", mode="before")
@classmethod
def normalize_delivered_at(cls, value: object) -> object:
return LoopItemResponse.normalize_unset_datetime(value)


class DeliveryDetailResponse(DeliveryResponse):
markdown: str
Expand Down
6 changes: 5 additions & 1 deletion backend/app/services/cloud_projects/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

"""Cloud project lifecycle and local execution bindings."""

import logging
import re
import uuid

Expand All @@ -27,6 +28,8 @@
)
from app.services.cloud_projects.access import require_cloud_project_role

logger = logging.getLogger(__name__)


class CloudProjectService:
def _generate_project_key(self, db: Session, name: str) -> str:
Expand Down Expand Up @@ -73,8 +76,9 @@ def create(
db.commit()
except IntegrityError as exc:
db.rollback()
logger.exception("Failed to create cloud project")
raise HTTPException(
status.HTTP_409_CONFLICT, "Cloud project key already exists"
status.HTTP_409_CONFLICT, "Cloud project could not be created"
) from exc
db.refresh(project)
return project
Expand Down
13 changes: 9 additions & 4 deletions backend/app/services/delivery/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@

from app.core.config import settings
from app.models.cloud_project import CloudProject, LoopItemTaskBinding
from app.models.delivery import Delivery, DeliveryAsset, LoopItem
from app.models.delivery import (
Delivery,
DeliveryAsset,
LoopItem,
loop_datetime_is_unset,
)
from app.schemas.base_role import BaseRole
from app.schemas.delivery import DeliveryCreate, LoopItemTaskBind
from app.services.delivery.access import require_loop_item_access
Expand Down Expand Up @@ -210,7 +215,7 @@ def finalize(self, db: Session, delivery_id: str, user_id: int) -> Delivery:
item = require_loop_item_access(
db, delivery.loop_item_id, user_id, BaseRole.Developer
)
if delivery.source_task_binding_id is not None:
if delivery.source_task_binding_id:
self._require_active_task_binding(
db, item.id, delivery.source_task_binding_id
)
Expand Down Expand Up @@ -333,7 +338,7 @@ def _resolve_source_task(
LoopItemTaskBinding.task_user_id == user_id,
LoopItemTaskBinding.device_id == source_task.device_id,
LoopItemTaskBinding.task_id == source_task.task_id,
LoopItemTaskBinding.unlinked_at.is_(None),
loop_datetime_is_unset(LoopItemTaskBinding.unlinked_at),
)
.first()
)
Expand All @@ -358,7 +363,7 @@ def _require_active_task_binding(
.filter(
LoopItemTaskBinding.loop_item_id == item_id,
LoopItemTaskBinding.id == binding_id,
LoopItemTaskBinding.unlinked_at.is_(None),
loop_datetime_is_unset(LoopItemTaskBinding.unlinked_at),
)
.first()
)
Expand Down
Loading
Loading