Skip to content

feat(api): allow deleting terminal bank operations with control-plane support#2777

Open
chethanuk wants to merge 3 commits into
vectorize-io:mainfrom
chethanuk:fix/issue-2677-delete-terminal-ops
Open

feat(api): allow deleting terminal bank operations with control-plane support#2777
chethanuk wants to merge 3 commits into
vectorize-io:mainfrom
chethanuk:fix/issue-2677-delete-terminal-ops

Conversation

@chethanuk

@chethanuk chethanuk commented Jul 17, 2026

Copy link
Copy Markdown

Problem

There is no way to remove terminal (failed / cancelled / completed) async operation rows. DELETE …/operations/{id} is bound to cancel, which only flips pending → cancelled and returns 409 for anything else — the row never leaves the table, so failed operations pile up in the control plane with no cleanup path and no navigation to the document they produced.

Fixes #2677

Fix

  • New dataplane route DELETE /v1/default/banks/{bank_id}/operations/{operation_id}/delete: status-guarded hard delete for terminal states only, implemented as a single bank-scoped DELETE … RETURNING (no TOCTOU window with a concurrent retry). 404 when missing, 409 for non-terminal states. The existing cancel route is untouched.
  • Engine: delete_operation + BankWriteOperation.DELETE_OPERATION (operation-validator aware).
  • Control plane: delete buttons in the operations table and detail dialog (terminal rows only) + a View Document action whenever result_metadata.document_id is present — the navigation path the issue reporter asked for.
  • OpenAPI spec and Python/TypeScript/Go client surfaces regenerated.
flowchart LR
    subgraph cp [Control plane]
        U["Operations table / dialog<br/>(Delete on terminal rows)"] --> C["client.deleteOperation()"]
        C --> B["BFF DELETE /api/banks/{bankId}/operations/{id}"]
    end
    B --> S["SDK deleteOperation"]
    S --> R["DELETE …/operations/{id}/delete"]
    R --> G{"status ∈ failed |<br/>cancelled | completed?"}
    G -->|yes| D["single bank-scoped<br/>DELETE … RETURNING"]
    G -->|"no (pending/processing)"| X["409 — use cancel<br/>(unchanged route)"]
    D --> P["row gone on next poll"]
Loading

Semantics: cancel vs delete

Verb Route Effect
Cancel DELETE …/operations/{id} pending → cancelled (soft, unchanged)
Delete record DELETE …/operations/{id}/delete hard-delete of a terminal row

A review bot suggested the control plane was calling the cancel verb — that's a mix-up of the BFF namespace with the dataplane path: the BFF's DELETE /api/banks/{bankId}/operations/{id} handler calls the generated SDK, which requests …/operations/{id}/delete; cancel lives on a separate BFF route (/api/operations/{bankId}).

Review fixes folded in

  • Python client surface properly regenerated — the previous hand-patch had a doubled @validate_call on delete_operation and had dropped the decorator from retry_operation (silently losing runtime arg validation there).
  • Route renamed …/operations/{operation_id}/record…/operations/{operation_id}/delete per maintainer review; OpenAPI spec and Go/Python/TypeScript clients regenerated (no client symbol names changed — they derive from the explicit operation_id="delete_operation", not the path).
  • "Failed to delete operation" registered in failureErrorKeys so error payloads localize like every sibling operation endpoint (keys already existed in all 10 locales).

How to test

cd hindsight-api-slim
uv run pytest tests/test_operation_status.py tests/test_operation_progress.py -v -n0

cd ../hindsight-control-plane
npm test && npm run i18n:check

Manual E2E: retain a doc so an operation ends failed → Operations → detail → View Document (when document_id set) → Delete → row disappears on next poll; cancel of a pending op still works via the old route.

Test evidence

Run post-rebase on upstream main (52b893b93), squashed branch:

Check Result
pytest tests/test_operation_status.py tests/test_operation_progress.py -n0 20 passed
control-plane npm test 76 passed (9 files)
npm run i18n:check clean
ruff check / format --check / ty check on touched modules clean
OpenAPI spec + Python/TS/Go clients regenerated via project scripts (openapi-generator 7.10.0 / @hey-api), no drift vs spec

Out of scope

@nicoloboschi nicoloboschi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

let's call the endpoint
/delete

@app.delete(
    "/v1/default/banks/{bank_id}/operations/{operation_id}/delete",

Maintainer review on vectorize-io#2777 asked for the hard-delete endpoint to live at
/delete rather than /record. Renames the path segment end-to-end:
dataplane route + log message, tests, OpenAPI spec (and its skills
mirror), and the generated Python/TypeScript/Go clients.

The route's operation_id is explicitly "delete_operation", so no
generated symbol names change -- only the path string. Go and Python
generated output was verified byte-identical against the pinned
openapi-generator v7.10.0.
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.

(Control Plane) Add route to delete Failed operation

2 participants