From ac4dee8579eecddd0a95ef1f76da65bd7a0cea6a Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Wed, 27 May 2026 23:32:59 -0700 Subject: [PATCH] Address Greptile review feedback for decomposition PR --- .../migrations/0090_demonic_grey_gargoyle.sql | 1 + .../db/src/migrations/meta/0090_snapshot.json | 24 +++- .../src/schema/issue_plan_decompositions.ts | 3 + .../pr-6831-plan-decomposition-completed.png | Bin 0 -> 5234 bytes .../pr-6831-plan-decomposition-in-flight.png | Bin 0 -> 5234 bytes server/src/__tests__/issues-service.test.ts | 1 + server/src/services/issues.ts | 19 ++- .../issue-plan-decompositions.stories.tsx | 129 ++++++++++++++++++ 8 files changed, 174 insertions(+), 3 deletions(-) create mode 100644 screenshots/pr-6831-plan-decomposition-completed.png create mode 100644 screenshots/pr-6831-plan-decomposition-in-flight.png create mode 100644 ui/storybook/stories/issue-plan-decompositions.stories.tsx diff --git a/packages/db/src/migrations/0090_demonic_grey_gargoyle.sql b/packages/db/src/migrations/0090_demonic_grey_gargoyle.sql index 404047d5f93..8c17a9b3d26 100644 --- a/packages/db/src/migrations/0090_demonic_grey_gargoyle.sql +++ b/packages/db/src/migrations/0090_demonic_grey_gargoyle.sql @@ -23,4 +23,5 @@ ALTER TABLE "issue_plan_decompositions" ADD CONSTRAINT "issue_plan_decomposition ALTER TABLE "issue_plan_decompositions" ADD CONSTRAINT "issue_plan_decompositions_owner_agent_id_agents_id_fk" FOREIGN KEY ("owner_agent_id") REFERENCES "public"."agents"("id") ON DELETE set null ON UPDATE no action;--> statement-breakpoint ALTER TABLE "issue_plan_decompositions" ADD CONSTRAINT "issue_plan_decompositions_owner_run_id_heartbeat_runs_id_fk" FOREIGN KEY ("owner_run_id") REFERENCES "public"."heartbeat_runs"("id") ON DELETE set null ON UPDATE no action;--> statement-breakpoint CREATE INDEX "issue_plan_decompositions_company_source_status_idx" ON "issue_plan_decompositions" USING btree ("company_id","source_issue_id","status");--> statement-breakpoint +CREATE INDEX "issue_plan_decompositions_active_owner_idx" ON "issue_plan_decompositions" USING btree ("company_id","owner_agent_id") WHERE "issue_plan_decompositions"."status" = 'in_flight';--> statement-breakpoint CREATE UNIQUE INDEX "issue_plan_decompositions_source_revision_uq" ON "issue_plan_decompositions" USING btree ("company_id","source_issue_id","accepted_plan_revision_id"); diff --git a/packages/db/src/migrations/meta/0090_snapshot.json b/packages/db/src/migrations/meta/0090_snapshot.json index 25169aa3f66..65026785085 100644 --- a/packages/db/src/migrations/meta/0090_snapshot.json +++ b/packages/db/src/migrations/meta/0090_snapshot.json @@ -10302,6 +10302,28 @@ "method": "btree", "with": {} }, + "issue_plan_decompositions_active_owner_idx": { + "name": "issue_plan_decompositions_active_owner_idx", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "owner_agent_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "where": "\"issue_plan_decompositions\".\"status\" = 'in_flight'", + "with": {} + }, "issue_plan_decompositions_source_revision_uq": { "name": "issue_plan_decompositions_source_revision_uq", "columns": [ @@ -18349,4 +18371,4 @@ "schemas": {}, "tables": {} } -} \ No newline at end of file +} diff --git a/packages/db/src/schema/issue_plan_decompositions.ts b/packages/db/src/schema/issue_plan_decompositions.ts index 47262eb0b3c..39ff5d97f71 100644 --- a/packages/db/src/schema/issue_plan_decompositions.ts +++ b/packages/db/src/schema/issue_plan_decompositions.ts @@ -36,6 +36,9 @@ export const issuePlanDecompositions = pgTable( table.sourceIssueId, table.status, ), + activeOwnerIdx: index("issue_plan_decompositions_active_owner_idx") + .on(table.companyId, table.ownerAgentId) + .where(sql`${table.status} = 'in_flight'`), sourceRevisionUq: uniqueIndex("issue_plan_decompositions_source_revision_uq").on( table.companyId, table.sourceIssueId, diff --git a/screenshots/pr-6831-plan-decomposition-completed.png b/screenshots/pr-6831-plan-decomposition-completed.png new file mode 100644 index 0000000000000000000000000000000000000000..c90422fe47ac6b590fe05c236ac63263bab8849d GIT binary patch literal 5234 zcmeHL`%jZs7=CGyi%6?DrKnswoy-{4y6NiDf;U!h*>EmScL5}JippG7EwsJ&2ZOD}Y%b4&IoEav-uIOpWtzW4IH z=RD{A{O!D)_=PJL0zmxE-0XJ%7N92Kmly)7wx+%j0k9Z$W^db5`L*?7CO7}!3W15E za*v)kkfwe&GWW>VjWOEQ)pfLGiipMWxzDAY&fb=tMsh}DZu!9Bb=~S}XUWY8dB=o0 zX~=(5#1sx)l8D9P@IU6Kx|gFWX0Z~LYhsE}nGi`wWh{XLAc%}ugakMPQ2=S%fC%tK zng9uti%-WbgM$DM5Bwy+&q(|__@7^tN`uL7SA^VSTz>G${ozDxeIHHg`%^4T$*gwW zVj&a$S@jD=xNNuFzN6WuCp_5dw^p5FzN9n3cn^`O=&tuc;^_9O*PELBg-f!FMe5U%;x`0#V9Jp%m82F)w z6^4i78KPqUGc} z1i>Y2q36+~N2a^=iFm5!hK2?taU)ecGGN$FRA#@b1p2U9B^(+Y zwCQC`Lgk#eja7BBriMlEIvmF%V68JW0Q7rZ#Y60%&1#jpACzd~&zU)D%BHB7K)D8y zrax{;rK@DOJ3BX&E6lgYmF+t)<%aSIID|M8U8U7(KN|@KgAz%saFntGyZR*gC#2a~ zq*-i7M@QQhzPz`$Dp;Hq+Q<2ovKgx`iiS6-f{3CsU0q$x-|=|7rR`%ZcH?P|l_CdV ztPKwh4-cOxknGj$VKSL~QZ#&`_5e{CX|Y($zAd%W_Off-^g=S~&kc7cCMIeWys~e~ zc3-b20%J{hhcZ!~0pg0F70o~-&q#d_=yx(CN4ZBj7r|RdtF=gaM`%}gJ@SPngVj-$ zc|3C!+@c~tEVbHfP1F7T{V$JH8HNF;Vtds|!H`vf61Jw2Ucr1Sy2f|zM5r?k|im+?5U0ff+V z(A_+ zLLq;C&Z77}Iwc3YbE0NZk_b~SfYy)9pYlZuAW)CMuM(#2FDz_2<~SQoW)Bp)jH zbF#ms(=RKv44O=)Cu9QL54%RbTfu4Wn@8H{8Oi919L8Cue7g^c|#{r z0R1vD$r!>?V0LO40nh2O#c|SE-F?%ylC{jbP0!EG&YsFwtvBymeZJPd1jZA>p?0`t zFpKB5_896`_WHlSa=yOxDxGxNtWP zj%%J0`4n0#;2a)*-8|zUP!eG4>1X|+N?m8=0Yt70=K}EeIi8BZlQeif>c5$~9~3Bc j7Yc=pViIaZg^vj$Y-CF{b3XDR)B`(T&&%%EUUcR!894{< literal 0 HcmV?d00001 diff --git a/screenshots/pr-6831-plan-decomposition-in-flight.png b/screenshots/pr-6831-plan-decomposition-in-flight.png new file mode 100644 index 0000000000000000000000000000000000000000..c90422fe47ac6b590fe05c236ac63263bab8849d GIT binary patch literal 5234 zcmeHL`%jZs7=CGyi%6?DrKnswoy-{4y6NiDf;U!h*>EmScL5}JippG7EwsJ&2ZOD}Y%b4&IoEav-uIOpWtzW4IH z=RD{A{O!D)_=PJL0zmxE-0XJ%7N92Kmly)7wx+%j0k9Z$W^db5`L*?7CO7}!3W15E za*v)kkfwe&GWW>VjWOEQ)pfLGiipMWxzDAY&fb=tMsh}DZu!9Bb=~S}XUWY8dB=o0 zX~=(5#1sx)l8D9P@IU6Kx|gFWX0Z~LYhsE}nGi`wWh{XLAc%}ugakMPQ2=S%fC%tK zng9uti%-WbgM$DM5Bwy+&q(|__@7^tN`uL7SA^VSTz>G${ozDxeIHHg`%^4T$*gwW zVj&a$S@jD=xNNuFzN6WuCp_5dw^p5FzN9n3cn^`O=&tuc;^_9O*PELBg-f!FMe5U%;x`0#V9Jp%m82F)w z6^4i78KPqUGc} z1i>Y2q36+~N2a^=iFm5!hK2?taU)ecGGN$FRA#@b1p2U9B^(+Y zwCQC`Lgk#eja7BBriMlEIvmF%V68JW0Q7rZ#Y60%&1#jpACzd~&zU)D%BHB7K)D8y zrax{;rK@DOJ3BX&E6lgYmF+t)<%aSIID|M8U8U7(KN|@KgAz%saFntGyZR*gC#2a~ zq*-i7M@QQhzPz`$Dp;Hq+Q<2ovKgx`iiS6-f{3CsU0q$x-|=|7rR`%ZcH?P|l_CdV ztPKwh4-cOxknGj$VKSL~QZ#&`_5e{CX|Y($zAd%W_Off-^g=S~&kc7cCMIeWys~e~ zc3-b20%J{hhcZ!~0pg0F70o~-&q#d_=yx(CN4ZBj7r|RdtF=gaM`%}gJ@SPngVj-$ zc|3C!+@c~tEVbHfP1F7T{V$JH8HNF;Vtds|!H`vf61Jw2Ucr1Sy2f|zM5r?k|im+?5U0ff+V z(A_+ zLLq;C&Z77}Iwc3YbE0NZk_b~SfYy)9pYlZuAW)CMuM(#2FDz_2<~SQoW)Bp)jH zbF#ms(=RKv44O=)Cu9QL54%RbTfu4Wn@8H{8Oi919L8Cue7g^c|#{r z0R1vD$r!>?V0LO40nh2O#c|SE-F?%ylC{jbP0!EG&YsFwtvBymeZJPd1jZA>p?0`t zFpKB5_896`_WHlSa=yOxDxGxNtWP zj%%J0`4n0#;2a)*-8|zUP!eG4>1X|+N?m8=0Yt70=K}EeIi8BZlQeif>c5$~9~3Bc j7Yc=pViIaZg^vj$Y-CF{b3XDR)B`(T&&%%EUUcR!894{< literal 0 HcmV?d00001 diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index 23f4f4f23b3..68fe6338ef0 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -3740,6 +3740,7 @@ describeEmbeddedPostgres("accepted plan decomposition", () => { expect(record?.childIssues.map((child) => child.id).sort()).toEqual( [...result.childIssueIds].sort(), ); + expect(record).not.toHaveProperty("requestedChildren"); expect(record?.childIssues.every((child) => typeof child.title === "string")).toBe(true); }); }); diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index b335ca2cac9..adf1c9c36bd 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -4414,9 +4414,24 @@ export function issueService(db: Db) { const childIssueMap = new Map(childIssueRows.map((row) => [row.id, row])); return rows.map((row) => { - const childIds = normalizeIssuePlanDecompositionChildIds(row.decomposition.childIssueIds); + const decomposition = row.decomposition; + const childIds = normalizeIssuePlanDecompositionChildIds(decomposition.childIssueIds); return { - ...row.decomposition, + id: decomposition.id, + companyId: decomposition.companyId, + sourceIssueId: decomposition.sourceIssueId, + acceptedPlanRevisionId: decomposition.acceptedPlanRevisionId, + acceptedInteractionId: decomposition.acceptedInteractionId, + status: decomposition.status, + requestFingerprint: decomposition.requestFingerprint, + requestedChildCount: decomposition.requestedChildCount, + childIssueIds: childIds, + ownerAgentId: decomposition.ownerAgentId, + ownerUserId: decomposition.ownerUserId, + ownerRunId: decomposition.ownerRunId, + completedAt: decomposition.completedAt, + createdAt: decomposition.createdAt, + updatedAt: decomposition.updatedAt, acceptedPlanRevisionNumber: row.revisionNumber ?? null, childIssues: childIds .map((childId) => childIssueMap.get(childId) ?? null) diff --git a/ui/storybook/stories/issue-plan-decompositions.stories.tsx b/ui/storybook/stories/issue-plan-decompositions.stories.tsx new file mode 100644 index 00000000000..e53f33c44f4 --- /dev/null +++ b/ui/storybook/stories/issue-plan-decompositions.stories.tsx @@ -0,0 +1,129 @@ +import { useState } from "react"; +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { useQueryClient } from "@tanstack/react-query"; +import type { AcceptedPlanDecompositionSummary } from "@paperclipai/shared"; +import { IssuePlanDecompositionsSection } from "@/components/IssuePlanDecompositionsSection"; +import { queryKeys } from "@/lib/queryKeys"; +import { storybookAgentMap } from "../fixtures/paperclipData"; + +const issueId = "issue-plan-decomposition-story"; +const issueIdentifier = "PAP-6831"; + +function buildDecomposition( + overrides: Partial, +): AcceptedPlanDecompositionSummary { + return { + id: "decomposition-story-1", + companyId: "company-storybook", + sourceIssueId: issueId, + acceptedPlanRevisionId: "revision-story-1", + acceptedInteractionId: "interaction-story-1", + status: "completed", + requestFingerprint: "fingerprint-story-1", + requestedChildCount: 2, + childIssueIds: ["issue-child-1", "issue-child-2"], + ownerAgentId: "agent-codex", + ownerUserId: null, + ownerRunId: "run-story-1", + completedAt: "2026-05-28T06:22:00.000Z", + createdAt: "2026-05-28T06:18:00.000Z", + updatedAt: "2026-05-28T06:22:00.000Z", + acceptedPlanRevisionNumber: 7, + childIssues: [ + { + id: "issue-child-1", + identifier: "PAP-6840", + title: "Harden accepted-plan wake routing", + status: "done", + priority: "medium", + assigneeAgentId: "agent-codex", + assigneeUserId: null, + }, + { + id: "issue-child-2", + identifier: "PAP-6841", + title: "Add decomposition regression coverage", + status: "in_progress", + priority: "medium", + assigneeAgentId: "agent-qa", + assigneeUserId: null, + }, + ], + ...overrides, + }; +} + +function HydratedSection({ + decompositions, +}: { + decompositions: AcceptedPlanDecompositionSummary[]; +}) { + const queryClient = useQueryClient(); + const [ready] = useState(() => { + queryClient.setQueryData(queryKeys.issues.acceptedPlanDecompositions(issueId), decompositions); + return true; + }); + + if (!ready) return null; + + return ( +
+
+
+ +
+
+
+ ); +} + +const meta = { + title: "Issue Detail/Plan Decompositions", + component: HydratedSection, + args: { + decompositions: [], + }, + parameters: { + layout: "fullscreen", + }, +} satisfies Meta; + +export default meta; + +type Story = StoryObj; + +export const InFlight: Story = { + args: { + decompositions: [ + buildDecomposition({ + status: "in_flight", + completedAt: null, + updatedAt: "2026-05-28T06:20:00.000Z", + childIssueIds: ["issue-child-1"], + childIssues: [ + { + id: "issue-child-1", + identifier: "PAP-6840", + title: "Harden accepted-plan wake routing", + status: "done", + priority: "medium", + assigneeAgentId: "agent-codex", + assigneeUserId: null, + }, + ], + }), + ], + }, +}; + +export const Completed: Story = { + args: { + decompositions: [ + buildDecomposition({}), + ], + }, +};