Skip to content

external_case_results: idempotency lookup (session_id, external_id) disagrees with DB constraint (project_id, external_id) #343

Description

@bg-playground

Problem

In external_case_results, the unique constraint is on (project_id, external_id):

__table_args__ = (
    UniqueConstraint("project_id", "external_id", name="uq_external_case_results_project_external_id"),
)

But the idempotency lookup in the POST handler is by (session_id, external_id). This means two different sessions in the same project submitting the same external_id will:

  • Pass the idempotency check (they have different session_id)
  • Attempt to insert a new row
  • Hit the DB UNIQUE constraint, which throws an IntegrityError/500

Relevant code (from PR #307)

Model definition:

# backend/app/models/external_results.py
class ExternalCaseResult(Base):
    __tablename__ = "external_case_results"
    __table_args__ = (
        UniqueConstraint("project_id", "external_id", name="uq_external_case_results_project_external_id"),
    )
    # ...

Auto-upsert (idempotency) check:

# backend/app/crud/external_results.py
if payload.external_id is not None:
    existing_result_q = await db.execute(
        select(ExternalCaseResult)
        .where(ExternalCaseResult.session_id == payload.session_id)
        .where(ExternalCaseResult.external_id == payload.external_id)
    )
    existing_result = existing_result_q.scalar_one_or_none()
    if existing_result is not None:
        # ... (returns existing)

Solution

  1. Decide what the true idempotency contract should be:
    • If results are project-global per external_id (likely):
      • Change the lookup to (project_id, external_id).
      • Return 200 and the existing row if found (like current session idempotency path).
      • Update docs to clarify.
    • If results are session-scoped per external_id,
      • The DB constraint should key on (session_id, external_id) instead.
      • Update migration and model accordingly.
  2. Add an explicit IntegrityError handler in the POST endpoint for now so collisions return 409 with a clear unique_violation code, not 500.
  3. Add regression test for this scenario (two sessions, same project, same external_id).

Acceptance criteria

  • Contract decision: idempotency matches constraint.
  • DB schema and CRUD are aligned.
  • Docs clarify which behavior is enforced.
  • Graceful error handling for collisions (never 500).
  • Regression test included.

Discovered during review of #307. Tagging external_results, area:external-results, and (likely) v0.2 milestone.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions