Skip to content

feat(batch): implement batch-runner MVP per PRP-33 (#280)#281

Open
w7-mgfcode wants to merge 1 commit into
devfrom
feat/prp-33-batch-runner-mvp
Open

feat(batch): implement batch-runner MVP per PRP-33 (#280)#281
w7-mgfcode wants to merge 1 commit into
devfrom
feat/prp-33-batch-runner-mvp

Conversation

@w7-mgfcode
Copy link
Copy Markdown
Owner

Summary

Implements the Batch Runner MVP per PRPs/PRP-33-batch-runner-mvp.md. One batch_job row fans out into N batch_job_item rows; each item runs sequentially via lazy delegation to JobService.create_job; per-item metrics land in the pinned five-key JSONB {wape, smape, mae, bias, sample_size}; the parent settles to completed | failed | partial.

Unblocks PRP-34 (parallel execution), PRP-35 (priority queue), PRP-36 (export and retry), PRP-37 (champion and heatmap) — the forward-compat columns + the partial picker index ship MVP-owned per PRP-33 § "Cross-Slice Coordination Matrix".

Closes #280. Refs #277.

What changed

  • Migration (alembic/versions/c1d2e3f40512_create_batch_tables.py): batch_job + batch_job_item tables, enums (BatchStatus, BatchOperation, BatchItemStatus), CHECK constraints (status / operation / priority band), FK CASCADE on batch_id, partial picker index with predicate exactly WHERE (status = 'pending'), plus 11 b-tree + 1 GIN index.
  • Models (app/features/batch/models.py): SQLAlchemy 2.0 Mapped[] columns mirroring the migration; valid-transition maps for parent + child.
  • Schemas (app/features/batch/schemas.py): Pydantic v2 with ConfigDict(strict=True) on request bodies + Field(strict=False) on the two date fields (the strict-mode policy linter at app/core/tests/test_strict_mode_policy.py discovers and approves them). BatchScope.kind and BatchSubmitRequest.operation use Literal[...] (not str enums) for strict-mode compatibility; BatchScopeKind is retained for response/internal callers.
  • Service (app/features/batch/service.py): submit / _expand_scope (handles manual, region, category, top_revenue, all) / _pick_next (compiles to FOR UPDATE SKIP LOCKED) / _execute_item (lazy-imports JobService per the cycle precedent at forecasting/service.py:786-787) / _shape_metrics (sample_size derived inside the batch slice from fold_metrics per PRP § "Why not 10" — Option (b) explicitly rejected as a vertical-slice violation) / _settle / get / list_items. structlog lifecycle events: batch.{created,item_started,item_completed,item_failed,completed}.
  • Routes (app/features/batch/routes.py): POST /batch/forecasting (202), GET /batch/{id}, GET /batch/{id}/items (allow-listed sort_by); 404s and the scope-cap 422 go through app/core/exceptions to RFC 7807 application/problem+json. Router wired into app/main.py between jobs_router and ingest_router.
  • Config: Settings.batch_max_scope_expansion = 1000 + matching .env.example row.
  • Frontend stub at /visualize/batch: types in frontend/src/types/api.ts, hooks in frontend/src/hooks/use-batches.ts (useSubmitBatch, useBatch with 2s poll on in-flight states, useBatchItems), placeholder page in frontend/src/pages/visualize/batch.tsx. Per PRP narrowing: no slider, no cancel button, no retry, no heatmap.

PRP-33 success criteria

  • alembic upgrade head on fresh Postgres creates both tables, every CHECK, and the partial picker index with predicate exactly WHERE (status = 'pending') — verified via pg_indexes query in test_migration_partial_index_present.
  • 3-pair manual backtest scope settles completed with completed_items=3; every item's metrics JSONB carries exactly {wape, smape, mae, bias, sample_size}test_submit_batch_happy_path.
  • grep -rn "for_update" app/features/batch/service.pyservice.py:245: .with_for_update(skip_locked=True).
  • Settings.agent_require_approval unchanged (still ["create_alias", "archive_run", "save_scenario"]).
  • app/features/jobs/, app/features/forecasting/, tests/test_e2e_demo.py unmodified (verified via git status).
  • All validation gates green: ruff check + ruff format --check + mypy app/ + pyright app/ + pytest -v -m "not integration" (1457 pass) + pytest -v -m integration (7 batch + e2e demo passing) + frontend pnpm tsc --noEmit + pnpm lint + pnpm vitest --run (119 pass).

Test plan

  • 25 backend unit tests (no DB) — models, schemas (strict-mode JSON path regression), service (pinned JSONB shape, picker SQL, manual cartesian, frozen params).
  • 7 backend integration tests (real Postgres) — happy path, partial failure, scope-cap 422, sort allow-list silent fallback, partial-index predicate, lifecycle event emission.
  • Non-regression: tests/test_e2e_demo.py still green.
  • Reviewer dogfoods /visualize/batch against a seeded DB.

🤖 Implementation per PRP-33; manual review encouraged on the _expand_scope direct-ORM-query approach (the PRP suggested lazy AnalyticsService/DimensionsService imports; this implementation uses direct data_platform.models queries — the data-platform-shared-orm-layer memory documents this as the de-facto shared ORM layer).

Ships the new app/features/batch/ vertical slice that orchestrates portfolio
forecasting batches. One batch_job row fans out into N batch_job_item rows;
each item runs sequentially via lazy delegation to JobService.create_job; per
item metrics land in the pinned five-key JSONB {wape, smape, mae, bias,
sample_size}; parent settles to completed | failed | partial.

What ships:

- Alembic migration creates batch_job + batch_job_item with every CHECK
  constraint, FK CASCADE, and the partial picker index
  WHERE (status = 'pending') that downstream-2 (priority queue) compiles
  against.
- BatchService with submit / _expand_scope / _pick_next (FOR UPDATE SKIP
  LOCKED wired) / _execute_item (lazy-imports JobService per the cycle
  precedent at forecasting/service.py:786-787) / _shape_metrics (sample_size
  derived inside the slice from fold_metrics — Option (a) per the PRP, not
  a reach into app/features/jobs/) / _settle / get / list_items.
- Three routes: POST /batch/forecasting (202), GET /batch/{id},
  GET /batch/{id}/items (allow-listed sort_by + RFC 7807 errors).
- BatchSubmitRequest uses ConfigDict(strict=True) + Field(strict=False) on
  start_date/end_date per docs/_base/SECURITY.md — the strict-mode policy
  linter discovers and approves the new fields.
- Forward-compat columns owned MVP-side per PRP-33 § "Cross-Slice
  Coordination Matrix": batch_job.{running_items, cancelled_items,
  max_parallel, default_child_priority} and batch_job_item.priority — the
  four downstream INITIALs consume these without further migrations.
- 25 unit tests cover the strict-mode JSON path regression, every
  BatchScope.kind, the pinned five-key metrics JSONB, and the picker SQL
  carrying FOR UPDATE SKIP LOCKED.
- 7 integration tests cover the happy path, partial failure, scope-cap
  422 (RFC 7807 problem+json), sort allow-list silent fallback, the
  partial-index predicate assertion, and the five-event structlog
  lifecycle emission (batch.{created,item_started,item_completed,item_failed,completed}).
- Settings.batch_max_scope_expansion = 1000 + .env.example entry.
- Frontend stub at /visualize/batch: TanStack Query hooks that poll the
  parent every 2s while in-flight, plus a minimal submit form + items table.
  Per PRP narrowing: no slider, no cancel, no retry, no heatmap.

Non-regression boundary (PRP-33 § "Success Criteria"):

- app/features/jobs/, app/features/forecasting/, tests/test_e2e_demo.py
  unchanged.
- Settings.agent_require_approval unchanged (MVP exposes zero mutating
  agent tools).
- All validation gates green: ruff check + ruff format --check + mypy
  --strict + pyright --strict + 1457 unit tests + 7 new integration tests
  + e2e demo + frontend tsc + lint + vitest (119 tests).

Unblocks PRP-34 (parallel execution), PRP-35 (priority queue), PRP-36
(export and retry), PRP-37 (champion and heatmap).
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 129c6476-43d7-43bf-ae7e-482e3585f1e0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/prp-33-batch-runner-mvp

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Sorry @w7-mgfcode, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

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