Skip to content

feat(postgres): GetStepProgress, EnqueueTx, UpdateRunSpec - #41

Merged
myzie merged 1 commit into
mainfrom
noodlespy-store-gaps
Apr 13, 2026
Merged

feat(postgres): GetStepProgress, EnqueueTx, UpdateRunSpec#41
myzie merged 1 commit into
mainfrom
noodlespy-store-gaps

Conversation

@myzie

@myzie myzie commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Three additive methods on *postgres.Store that unblock consumers migrating off hand-rolled Postgres adapters onto the experimental store. None touch the worker package, the runquery interface, or existing method signatures.

  • GetStepProgress(ctx, executionID) ([]workflow.StepProgress, error) — reads step_progress rows back out. Ordered by started_at NULLS LAST, then step_name, then branch_id. Lives on *Store, not runquery.Store, to keep the backend-neutral read interface minimal. Consumers whose runs come back from runquery.Store.GetRun previously had no API to render per-step status in their UI.

  • EnqueueTx(ctx, tx pgx.Tx, run) error — transaction-aware enqueue so the run insert can be atomic with writes to adjacent tables (credit ledger debits, idempotency keys, audit rows) that live outside the library's schema but in the same database. Enqueue is now a one-line wrapper around EnqueueTx with its own transaction. pgx.Tx is the right type here — experimental/store/postgres already exposes *pgxpool.Pool publicly, so this does not leak a new dependency.

  • UpdateRunSpec(ctx, claim, spec) error — replaces the spec on a running claim with the same (claim_id, worker_id, attempt, status=running) fencing as Heartbeat/Complete, returning ErrLeaseLost on fence failure. For long-running activities that mutate the spec incrementally (e.g. a KB-apply loop persisting progress between steps) and need the update durable without waiting for the next checkpoint.

Test plan

  • TestStore_GetStepProgress — ordering (started_at asc, NULLS last), detail round-trip, per-execution isolation, status parsing
  • TestStore_EnqueueTxRollbackAndCommit — tx rollback leaves no row; commit with cross-table ledger write is atomic; nil tx rejected
  • TestStore_UpdateRunSpecFencing — happy path updates spec; wrong worker → ErrLeaseLost; wrong attempt → ErrLeaseLost; completed run → ErrLeaseLost; nil claim rejected
  • make test-all green (root module + worker + postgres store + sqlite store)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Workflow enqueuing now uses transaction-based persistence with automatic rollback on failure
    • New methods enable updating workflow specifications during execution with lease validation
    • Added step progress retrieval for workflow executions with proper ordering and null handling
  • Testing

    • Extended test suite covering transaction behavior, lease fencing, and data isolation

Three additive methods on *postgres.Store that unblock consumers
migrating off hand-rolled Postgres adapters onto the experimental
store. None of them touch the worker package, the runquery
interface, or existing method signatures.

- GetStepProgress(ctx, executionID) reads the step_progress rows
  back out. Without it, consumers whose runs come through
  runquery.Store.GetRun have no API to render per-step status in
  their UI. Ordered by started_at NULLS LAST, step_name, branch_id.
  Lives on *Store, not runquery.Store, to keep the backend-neutral
  read interface minimal.

- EnqueueTx(ctx, tx, run) takes a caller-owned pgx.Tx so the run
  insert can be atomic with writes to adjacent tables (credit
  ledger debits, idempotency keys, audit rows) that live outside
  the library's schema. Enqueue is now a one-line wrapper around
  EnqueueTx with its own transaction. pgx.Tx is the right type
  here — experimental/store/postgres already exposes
  *pgxpool.Pool publicly, so this does not leak a new dependency.

- UpdateRunSpec(ctx, claim, spec) replaces the spec on a running
  claim with the same (claim_id, worker_id, attempt, status=running)
  fencing as Heartbeat and Complete, returning ErrLeaseLost on
  fence failure. For long-running activities that mutate the spec
  incrementally (e.g. a KB-apply loop persisting progress between
  steps) and need it durable without waiting for the next
  checkpoint.

Tests exercise ordering and detail round-trip for GetStepProgress,
rollback+commit semantics and cross-table atomicity for EnqueueTx,
and all three fence failure modes for UpdateRunSpec (wrong worker,
wrong attempt, completed run). Nil-argument guards on EnqueueTx
and UpdateRunSpec.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Apr 13, 2026

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The changes introduce transaction-based enqueueing for workflow runs with a new delegated method (EnqueueTx), add lease-fenced spec updates for running claims (UpdateRunSpec), and implement step progress retrieval from the database. Comprehensive integration tests validate transaction rollback/commit, lease fencing semantics, and result ordering.

Changes

Cohort / File(s) Summary
Queue Store Transactions & Fencing
experimental/store/postgres/queue.go
Modified Enqueue to wrap inserts in a pgx transaction and delegate to new EnqueueTx method. Added EnqueueTx to insert runs using caller-provided transactions. Added UpdateRunSpec to update run specs with lease-fence conditions (worker ID, attempt, running status) and return worker.ErrLeaseLost on fence violations.
Step Progress Retrieval
experimental/store/postgres/step_progress.go
Added GetStepProgress method to query and hydrate workflow.StepProgress objects from the database, including status enum conversion, optional JSON detail unmarshaling, and nullable timestamp handling; ordered by started_at (NULLs last), step_name, and branch_id.
Integration Tests
experimental/store/postgres/store_test.go
Added tests validating GetStepProgress (empty slices for missing executions, detail round-tripping, per-execution isolation, result ordering, pending timestamps), EnqueueTx transaction isolation and nil-tx rejection, and UpdateRunSpec lease fencing (correct/incorrect worker ID/attempt, post-completion rejection, nil claim rejection).

Sequence Diagrams

sequenceDiagram
    participant Client
    participant Store
    participant PgxPool as PgxPool<br/>(Begin)
    participant PgxTx as PgxTx<br/>(Insert)
    participant Database

    Client->>Store: Enqueue(ctx, run)
    Store->>PgxPool: Begin(ctx)
    PgxPool->>Database: Start Transaction
    Database-->>PgxTx: Transaction Handle
    Store->>Store: EnqueueTx(ctx, tx, run)
    Store->>PgxTx: Exec(insert into workflow_runs)
    PgxTx->>Database: INSERT workflow_runs
    Database-->>PgxTx: Success
    PgxTx-->>Store: RowsAffected
    Store->>PgxTx: Commit(ctx)
    PgxTx->>Database: Commit Transaction
    Database-->>Client: Success
Loading
sequenceDiagram
    participant Client
    participant Store
    participant Database

    Client->>Store: UpdateRunSpec(ctx, claim, spec)
    Note over Store: Validate claim fields<br/>(WorkerID, Attempt)
    Store->>Database: UPDATE workflow_runs SET spec = ?<br/>WHERE id = ? AND claimed_by = ?<br/>AND attempt = ? AND status = 'running'
    Database-->>Store: RowsAffected
    alt RowsAffected > 0
        Store-->>Client: Success (spec updated)
    else RowsAffected == 0
        Store-->>Client: worker.ErrLeaseLost<br/>(lease fence violated)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 Transactions dance with grace so fine,
Each enqueue wrapped in threads that align,
Leases fenced 'gainst unruly claims,
Steps progress traced through database lanes,
Rollbacks rewind, commits make it stick—
The queue hops safer, swift and quick!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main changes by listing the three new methods added (GetStepProgress, EnqueueTx, UpdateRunSpec), which aligns with the changeset's primary objective.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch noodlespy-store-gaps

Comment @coderabbitai help to get the list of available commands and usage tips.

@myzie
myzie merged commit b0e3e36 into main Apr 13, 2026
1 of 2 checks passed
@myzie
myzie deleted the noodlespy-store-gaps branch April 13, 2026 02:51
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.

1 participant