Skip to content

ART M6: approval and transactional staged import (#110) - #227

Merged
amar-python merged 3 commits into
claude/issue-109-dedup-versioned-datasetsfrom
claude/issue-110-approval-staged-import
Nov 10, 2026
Merged

amar-python merged 3 commits into
claude/issue-109-dedup-versioned-datasetsfrom
claude/issue-110-approval-staged-import

Conversation

@amar-python

Copy link
Copy Markdown
Owner

What

M6 of the ART plan: approval and a transactional staged import (BR-32, BR-33, BR-34; ADR-0002; decisions 4 and 5). Tracker: #110.

Stacked on #226 (M5). The base is the M5 branch, so this diff shows only M6. Merge #226 first and GitHub will retarget this to main.

For dynamic-mode uploads the flow is now stage → review → submit → approve or reject → import:

  • Stage (POST /api/csv/uploads): stores the original and parsed rows; nothing is imported. Duplicate-row flags against earlier versions (M5) are computed here, so the reviewer sees them before approving.
  • Submit / approve / reject (POST /api/csv/uploads/{id}/submit, /approval): a rejection needs a reason, and a rejected upload can be reviewed again and resubmitted. Approving needs every row that isn't excluded to validate.
  • Self-approval (decision 5): a signed-in member can't approve data they uploaded or submitted. A tenant with one user turns it on with accounts.set_self_approval or scripts/create_tenant_admin.py --allow-self-approval. A request with no session has no identity to compare, so the rule doesn't apply to it.
  • Approval snapshots the review. It records the newest row decision and cell edit at that moment; the import applies exactly those, and review is refused once approved. What was approved is what is imported.
  • Import (POST /api/csv/uploads/{id}/import, Idempotency-Key header): one transaction COPYs the approved rows into a temporary staging table, promotes them into csv_<slug>_v<n> with _source_row_id and _import_id, registers the file, writes the imports record and the audit row, and commits. Any failure rolls back all of it. A repeat with the same key returns the original result; another key for an imported upload is a 409.
  • Reconciliation is stored per import (rows in, excluded, edited, duplicate, imported) and the database checks rows in = excluded + duplicate + imported.
  • Migration 0011: submissions, approvals, imports (insert-only, and in APPEND_ONLY so the runtime role can't change them), the staged status, uploads.types_provenance (Dynamic mode: confirm inferred types before table creation, with a provenance flag #195), and tenants.allow_self_approval.

Decisions to check

  • Dynamic mode only. T&E mode and the direct POST /api/csv/upload are unchanged, so they still import without approval, with row-level isolation. VCRM keeps BR-32, BR-33 and BR-34 at Partial for that reason. Moving T&E over needs a stored target table, DB-constraint handling and T&E review typing; I'd do it as its own piece.
  • The audit gap is closed for the new flow only. The tracker says M6 closes the post-commit audit gap; the direct upload path still writes its audit row afterwards. README, the audit and originals docstrings, and ADR-0002 now say exactly that.
  • Validation is in Python before COPY, not in SQL after it (ADR-0002 said SQL). It uses the same cast_value contract as the loaders and the frontend, and a failing row stops approval, so nothing invalid reaches staging. Recorded in an implementation note on the ADR.
  • UNIQUE (upload_id) on imports, stricter than the ADR's (upload_id, approval_id): an upload is imported once, ever.
  • Failed imports leave no record, by design: the import record is written inside the transaction, so it exists only if the data does.
  • Two staged uploads of the same bytes are allowed to coexist; whichever imports second gets a 409 and stays approved but unimportable.
  • New roles: submit_import (uploader, tenant admin), approve_import (reviewer, tenant admin), run_import (uploader, reviewer, tenant admin). docs/ROLES.md and the role-matrix test cover them.

Not in this PR

  • Frontend. No UI for staging, submitting, approving or importing yet; the API is complete and tested.
  • The M6 branch history contains one wip: commit from before I merged main; squash on merge.

Verification

Full suite (everything except the browser job) against PostgreSQL 16, built as CI builds it: all 525 tests pass. scripts/lint.sh clean, bandit shows only the existing api/db.py notes, docs/TEST_CASES.md regenerated.

New: tests/system/test_staged_import.py (24 tests) covers the tracker's list: reject then resubmit; failure injected at three steps of the import, each followed by a successful same-key retry; a retry after success; concurrent imports with different and with the same key. It also covers reconciliation and lineage, data fidelity through COPY (quotes, \N, newlines, unicode, NULLs, typed columns), self-approval, and the workflow tables being insert-only. Five migration tests cover 0011, and the staged import runs as the restricted runtime role. Breaking the self-approval rule, or committing before the import finishes, makes the relevant tests fail.

#110 stays open until this merges.


Generated by Claude Code

…3, BR-34; ADR-0002)

For dynamic-mode uploads: stage, review, submit, approve or reject, then import
in one transaction.

- POST /api/csv/uploads stores the original and parsed rows; nothing is imported.
- Submit, then a reviewer approves or rejects (a rejection needs a reason; a
  rejected upload can be reviewed again and resubmitted). A signed-in member
  can't approve data they uploaded or submitted unless the tenant allows it.
- Approval snapshots the review (newest decision and edit ids), and review is
  refused once approved, so what was approved is what is imported.
- Import (Idempotency-Key header): COPY into a temporary staging table, promote
  into csv_<slug>_v<n> with _source_row_id and _import_id, register the file, write
  the imports record and the audit row, commit. Any failure rolls all of it back.
  A repeat with the same key returns the original; another key is a 409.
- Reconciliation is stored per import and checked by the database:
  rows_in = excluded + duplicate + imported.
- Migration 0011: submissions, approvals, imports (insert-only), the staged status,
  uploads.types_provenance, and tenants.allow_self_approval (off by default).
- The direct /api/csv/upload path and T&E mode are unchanged.

Merges the updated M5 branch (which carries current main): migration renumbered
0010 -> 0011, tests moved to tests/system/.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011T6WqhUfEMZv2Dxerqs2Eq
The CORS allow-list lacked Idempotency-Key, so a browser's preflight for
POST /api/csv/uploads/{id}/import was refused with 400 and the import could
only be called by scripts. Adds the header and a preflight test.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011T6WqhUfEMZv2Dxerqs2Eq
@amar-python
amar-python marked this pull request as ready for review November 10, 2026 03:14
@amar-python
amar-python merged commit dac5dd4 into claude/issue-109-dedup-versioned-datasets Nov 10, 2026
4 checks passed
@amar-python
amar-python deleted the claude/issue-110-approval-staged-import branch November 10, 2026 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants