Skip to content

perf(tasks): maintain task browse summaries on trial writes - #1185

Merged
kyle-compute merged 1 commit into
stagingfrom
perf/tasks-browse-last-activity
Aug 12, 2026
Merged

perf(tasks): maintain task browse summaries on trial writes#1185
kyle-compute merged 1 commit into
stagingfrom
perf/tasks-browse-last-activity

Conversation

@kyle-compute

@kyle-compute kyle-compute commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Why this PR exists

  • Loading the main /tasks page currently does database work proportional to the organization's trial history even though the screen shows only 24 task cards.
  • Fixing that requires an indexed per-version summary that the page query can order and limit on before touching trial rows.
  • This PR adds that summary's model, helpers, and write-through refresh hooks. It deliberately changes no read path.
  • The combined re-land (perf(tasks): bound task browser page work #1152) was 936 lines of application code; the limit is 500. This is split 1 of 4.

Split structure and merge order

  1. perf(tasks): maintain task browse summaries on trial writes #1185 (this PR) — summary maintenance: model, helpers, write-through refresh hooks. 432 application-code lines.
  2. perf(tasks): repopulate historical task browse summary rows #1190 — historical population: migration task_browse_summary_002 rebuilds every summary row.
  3. perf(tasks): summary-first /tasks/browse selection with bounded previews #1191 — backend read cutover: GET /tasks/browse becomes summary-first with bounded previews. 452 application-code lines.
  4. perf(frontend): render bounded task-card trial preview #1192 — frontend card cap: render the bounded preview and "showing 24 of N". 48 application-code lines.
  • Merge strictly in that order. Each later PR is based on the previous PR's branch; after a parent merges, rebase the child onto staging and retarget it before merging.
  • Each piece gets its own preview deployment, staging verification, and rollback.

Exact screen, request, and frequency

  • Screen: the authenticated main task-card grid at /tasks.
  • Screenshot gate: attach a representative slow /tasks skeleton and loaded grid to this body before merge.
  • Controlling browser request: GET /api/tasks/browse. This PR does not change its behavior; it prepares the data that perf(tasks): summary-first /tasks/browse selection with bounded previews #1191 will read.
  • Write frequency: every trial create/import, start/reset, completion, cancellation, retry/supersede, scoped deletion, experiment deletion, and default-version selection refreshes the affected version's summary row inside the same transaction.
  • S3 involvement: none.

Production evidence

  • Production p75 for /api/tasks/browse was approximately 8.86 seconds; production /tasks LCP was approximately 9.15 seconds.
  • The main task-selection SQL alone took approximately 3.09 seconds because it groups the organization's entire trial history before applying the 24-task limit.
  • The summary rows this PR maintains are what lets perf(tasks): summary-first /tasks/browse selection with bounded previews #1191 order and limit first instead.

What this PR changes

  • Adds TaskBrowseSummaryModel over the existing task_version_browse_summaries table: one row per task version with exact card counters, status buckets, model-grouped cost inputs, and last_run_at. The table already exists in staging and production (migration task_browse_summary_001 survived the Revert "perf(tasks): bound task browser page work" #1156 revert), so this PR contains no DDL.
  • Adds refresh_task_browse_summaries and calls it in the same transaction at every trial-population mutation: create/import, start/reset, completion, cancellation, retry/supersede, scoped deletion, experiment deletion, and default-version selection.
  • Serializes refreshes with sorted transaction-scoped advisory locks; concurrent trial inserts hold KEY SHARE FK locks on task_versions, and a FOR UPDATE upgrade there can deadlock (documented in AGENTS.md).
  • Ships the shared cost helper with the corrected billed_cost_usd accumulator key. The first landing wrote billed_usd, which is the KeyError that 500'd /tasks for any org with a billed trial and caused the Revert "perf(tasks): bound task browser page work" #1156 revert.
  • No reader change: GET /tasks/browse still aggregates on demand.
  • No historical scan: no migration, no backfill. Rows exist only for versions written to after deploy.
  • Safe to deploy unused: nothing reads the rows yet.

Why this is being re-landed

What this PR does not change

Historical-data decision

Validation completed

  • backend tests/test_browse_search.py: 5 passed. The single failure, test_search_grammar_and_or_exclude, fails identically on clean staging.
  • New test: experiment delete refreshes the surviving task's summary row in the delete transaction (the Bugbot finding on the combined PR).
  • New test: two sessions insert trials for the same version (both holding KEY SHARE FK locks), then refresh; the holder must not block and the second refresh must serialize behind the first's commit. A lock-ordering bug hangs this test instead of flaking it.
  • oddish tests/test_retry_trial_core.py and tests/test_local_runner_gate.py: 20 passed.

Rollback

  • Revert the hooks. Users never saw a behavior change, because nothing reads the summary rows.
  • The table stays; it already exists in production and is harmless unread.

@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
oddish-app Ready Ready Preview Aug 12, 2026 12:37am

Comment thread oddish/src/oddish/core/endpoints/deletion.py

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Risk: high. Left a non-blocking comment — Cursor Bugbot finished without a clean pass and left an unresolved high-severity finding, so this was not approved. Reviewers were assigned for human review.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Oddish preview

Commit: bf1223bf8609264d9277503ebfb725faa2eff31f

Surface Link Target
Frontend https://pr-1185.oddish.app Vercel preview for bf1223b
Backend oddish-pr-1185 oddish-pr-1185
Database project lxfqpxpcttlaqlhfwsoh project lxfqpxpcttlaqlhfwsoh

Vercel deployment URL: https://oddish-7goxerv0n.oddish.app

Plan:

  • Frontend deploy: true
  • Backend deploy: true
  • Migrations: false

This comment is updated by the PR Preview workflow.

Comment thread frontend/src/app/(app)/tasks/task-card.tsx Outdated
One summary row per task version (exact card counters, status buckets,
model-grouped cost inputs, last_run_at), refreshed inside the same
transaction as every trial-population mutation: create/import,
start/reset, completion, cancellation, retry/supersede, scoped deletion,
experiment deletion, and default-version selection. Refreshes serialize
per version with sorted transaction-scoped advisory locks because
concurrent trial inserts hold KEY SHARE FK locks on task_versions.

No reader change and no historical scan: the browse endpoint still
aggregates on demand, and rows exist only for versions written to after
deploy. The read cutover and historical population land separately.

Split 1/4 of the #1152 re-land.
@kyle-compute
kyle-compute force-pushed the perf/tasks-browse-last-activity branch from 514a235 to bf1223b Compare August 12, 2026 00:28

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit bf1223b. Configure here.

Comment thread oddish/src/oddish/core/endpoints/task_detail.py
@kyle-compute kyle-compute changed the title perf(tasks): re-land bounded task browser page work perf(tasks): maintain task browse summaries on trial writes Aug 12, 2026
@kyle-compute
kyle-compute merged commit b34c1a0 into staging Aug 12, 2026
18 checks passed
@kyle-compute
kyle-compute deleted the perf/tasks-browse-last-activity branch August 12, 2026 01:05
pfbyjy pushed a commit that referenced this pull request Aug 12, 2026
Four tests broke against code that arrived from staging, not against
this branch's changes: the browse-summary refresh (#1185) added a
(id, task_version_id) select and an advisory-lock execute that the
deletion and task-detail fakes did not model, the ephemeral Harbor
runner grew a required environment_config argument, and the cleanup
sweep test still patched the deleted _reset_orphaned_trial_analysis.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GzxfgtotWcStMXaRAkvotj
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