feat: group scheduled task results#977
Conversation
|
Claude (@claude) review |
|
Greptile (@greptileai) review |
|
Claude Code is working… I'll analyze this and get back to you. |
✅ Tests passed — 1224/1228
|
Greptile SummaryThis PR refactors the scheduled-task results panel from a flat chronological list into grouped, single-open collapsible sections — one per scheduled task — showing the task name, result count, and latest run status in the header. A new
Confidence Score: 5/5Safe to merge — grouping logic is well-isolated, the accordion state machine handles all edge cases, and the button-event refactor correctly replaces stopPropagation with structural separation. The change is self-contained: new utility logic is covered by targeted tests, the UI state derivation handles initialization/stale-group/user-closed branches correctly, and no existing behavior is regressed. No data paths, storage calls, or cross-component contracts are affected. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["ScheduledTaskResults renders"] --> B["groupScheduledTaskRuns(runs, jobs)"]
B --> C["Build jobsById Map"]
C --> D["Group runs by jobId → groupsByJobId Map"]
D --> E["For each group:\nsortedRuns = compareRunsForDisplay\nlatestRun = compareRunsByNewest[0]"]
E --> F["filter(group.latestRun exists)"]
F --> G["sort groups by latestRun.startedAt desc"]
G --> H["taskGroups returned"]
H --> I["Derive visibleExpandedGroupId"]
I -->|"undefined (initial)"| J["taskGroups[0].id"]
I -->|"expandedGroupId not in groups"| J
I -->|"null (user closed)"| K["null — all collapsed"]
I -->|"valid id"| L["expandedGroupId"]
J --> M["Render Collapsibles\none open at a time"]
K --> M
L --> M
M --> N["CollapsibleTrigger: name, count, latest status"]
M --> O["CollapsibleContent: per-run rows"]
O --> P["button → onViewRun (content area)"]
O --> Q["Cancel / Retry / View buttons (sibling div)"]
Reviews (2): Last reviewed commit: "feat: group scheduled task results (bosm..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| {group.runs.map((run) => { | ||
| const preview = getRunPreview(run) | ||
|
|
||
| return ( | ||
| <div | ||
| key={run.id} | ||
| className="flex items-start gap-3 rounded-lg border border-border bg-background p-3" | ||
| > | ||
| <div className="pt-0.5">{getStatusIcon(run.status)}</div> | ||
| <button | ||
| type="button" | ||
| onClick={() => onViewRun(run)} | ||
| className="min-w-0 flex-1 text-left" | ||
| > | ||
| <div className="flex flex-wrap items-center gap-2"> | ||
| <span className="text-foreground text-sm"> | ||
| {formatRunTimestamp(run.startedAt)} | ||
| </span> | ||
| <span className="text-muted-foreground text-xs"> | ||
| {run.status} | ||
| </span> | ||
| </div> | ||
| {preview && ( | ||
| <p className="mt-1 line-clamp-2 text-muted-foreground text-xs"> | ||
| {preview} | ||
| </p> | ||
| )} | ||
| </button> | ||
| <div className="flex shrink-0 items-center gap-1"> | ||
| {run.status === 'running' && ( | ||
| <Button | ||
| variant="ghost" | ||
| size="icon-sm" | ||
| onClick={() => onCancelRun(run.id)} | ||
| className="text-destructive hover:bg-destructive/10 hover:text-destructive" | ||
| aria-label="Cancel run" | ||
| > | ||
| <Square className="h-3.5 w-3.5" /> | ||
| </Button> | ||
| )} | ||
| {run.status === 'failed' && ( | ||
| <Button | ||
| variant="ghost" | ||
| size="icon-sm" | ||
| onClick={() => onRetryRun(run.jobId)} | ||
| className="text-muted-foreground hover:text-foreground" | ||
| aria-label="Retry run" | ||
| > | ||
| <RotateCcw className="h-3.5 w-3.5" /> | ||
| </Button> | ||
| )} | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| onClick={() => onViewRun(run)} | ||
| className="text-muted-foreground hover:text-foreground" | ||
| > | ||
| View | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| ) | ||
| })} |
There was a problem hiding this comment.
The previous implementation explicitly floated running runs before completed/failed ones. In the new grouping, runs are sorted purely by startedAt (newest-first), so a currently-running task that started earlier than a recently-completed run will appear below it in the list. In practice this edge case is rare, but it's a behavioral regression worth confirming is intentional.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browseros-agent/apps/agent/entrypoints/app/scheduled-tasks/ScheduledTaskResults.tsx
Line: 144-206
Comment:
**Running tasks no longer sorted to the top within a group**
The previous implementation explicitly floated `running` runs before `completed`/`failed` ones. In the new grouping, runs are sorted purely by `startedAt` (newest-first), so a currently-running task that started earlier than a recently-completed run will appear below it in the list. In practice this edge case is rare, but it's a behavioral regression worth confirming is intentional.
How can I resolve this? If you propose a fix, please make it concise.
Nikhil (shadowfax92)
force-pushed
the
polecat/jade/bosmain-obt@moxnpvqc
branch
from
764b730 to
d405f59
Compare
|
Addressed the Greptile notes in d405f59: default expanded group is now derived before paint instead of set in an effect, and running runs are kept first within a group while latest-run/group ordering still uses startedAt. Local gates rerun: typecheck, test, lint, build. |
|
Greptile (@greptileai) review |
1 participant
Summary
Test plan
bun run --filter @browseros/agent typecheckbun run --filter @browseros/agent testbun run --filter @browseros/agent lint(exits 0; reports existing unrelated warnings in NewTab, Sentry sanitize, and tool-labels)bun run --filter @browseros/agent buildFixes #950