feat(FN-5252): update done-card files chip display logic
Adds a new files chip on done-state TaskCards showing insertion/deletion counts, backed by new types (`filesChanged`, `insertions`, `deletions`) in core; architecture docs updated to reflect the change. The component implementation is a net reduction of 20 lines, with corresponding test coverage add Fusion-Task-Id: FN-5252
This commit is contained in:
committed by
gsxdsm
parent
c4745caec0
commit
61dd439e20
@@ -1384,13 +1384,16 @@ Done-task file-count surfaces intentionally distinguish three data sources:
|
||||
|
||||
1. **`/api/tasks/:id/diff` (lineage union, authoritative landed diff)**
|
||||
- This route aggregates the task's landed lineage and returns `stats.filesChanged` plus the file list used by the Changes tab.
|
||||
- Task cards and done-task diff views should treat this as the canonical "files changed" source.
|
||||
- Done-task cards and diff views should treat this as the canonical "files changed" source.
|
||||
2. **`task.mergeDetails.filesChanged` / `insertions` / `deletions` (final-commit shortstat)**
|
||||
- These fields describe only the recorded final merge/squash commit shortstat.
|
||||
- In multi-commit lineages this can undercount the full landed diff and is therefore labeled as commit-level metadata (for example, "Files in merge commit" / "Final commit summary").
|
||||
3. **`task.modifiedFiles` (execution-time worktree snapshot)**
|
||||
- On done cards, `mergeDetails.filesChanged` is only a transient loading placeholder until `/api/tasks/:id/diff` resolves.
|
||||
3. **`task.mergeDetails.landedFiles` (recorded committed file list)**
|
||||
- When live diff stats are unavailable, done-task cards may fall back to the recorded landed file list length.
|
||||
- This remains committed-diff metadata; transient executor worktree captures are not surfaced as a done-card files chip.
|
||||
4. **`task.modifiedFiles` (execution-time worktree snapshot)**
|
||||
- Captured in the executor worktree during implementation (`git diff <base>..HEAD` snapshot), before final merge outcomes are known.
|
||||
- Can include transient/superset paths that did not land; UI labels this as "files touched during execution" rather than "files changed" for done tasks.
|
||||
- Can include transient/superset paths that did not land; done-task cards must not use it for the files-changed chip.
|
||||
|
||||
**FN-4647 decision:** `mergeDetails` shortstat fields remain commit-level metadata. No additional persisted lineage-level summary field is introduced at this time; done-task landed totals continue to be served live via `/api/tasks/:id/diff`.
|
||||
|
||||
|
||||
@@ -1573,10 +1573,10 @@ export interface Task {
|
||||
* task worktree (`TaskExecutor.captureModifiedFiles`).
|
||||
*
|
||||
* This may be a stale/transient superset of files that actually landed after
|
||||
* merge resolution or follow-up commits. UI surfaces must label this as
|
||||
* "files touched during execution" (never landed "files changed" for done
|
||||
* tasks). The authoritative landed diff for done tasks is
|
||||
* `/api/tasks/:id/diff`.
|
||||
* merge resolution or follow-up commits. Done-task cards must not use this
|
||||
* field for their files-changed chip; the authoritative landed diff comes
|
||||
* from `/api/tasks/:id/diff`, with `mergeDetails.landedFiles` as committed
|
||||
* metadata fallback when live stats are unavailable.
|
||||
*/
|
||||
modifiedFiles?: string[];
|
||||
/** Opt out of the squash file-scope invariant for this task. */
|
||||
|
||||
@@ -1522,11 +1522,8 @@ function TaskCardComponent({
|
||||
}
|
||||
|
||||
if (task.column === "done") {
|
||||
// Per FN-4527/FN-4647: /api/tasks/:id/diff is authoritative for done-task
|
||||
// landed file counts. mergeDetails.filesChanged can be stale after
|
||||
// rebase-and-push (FN-4526), so only use it as a transient loading
|
||||
// placeholder. Executor-captured modifiedFiles are labeled as
|
||||
// "touched during execution" and never as landed "files changed".
|
||||
// Done cards only display committed diff counts from authoritative lineage
|
||||
// stats or recorded landed files; transient execution-touched files are not shown.
|
||||
let displayCount: number | undefined;
|
||||
if (diffStats) {
|
||||
const landed = task.mergeDetails?.landedFiles;
|
||||
@@ -1537,7 +1534,7 @@ function TaskCardComponent({
|
||||
} else if (diffLoading) {
|
||||
displayCount = task.mergeDetails?.filesChanged ?? undefined;
|
||||
} else {
|
||||
displayCount = undefined;
|
||||
displayCount = task.mergeDetails?.landedFiles?.length;
|
||||
}
|
||||
if (displayCount != null && displayCount > 0) {
|
||||
return (
|
||||
@@ -1552,25 +1549,6 @@ function TaskCardComponent({
|
||||
</button>
|
||||
);
|
||||
}
|
||||
|
||||
const landedFallbackCount = task.mergeDetails?.landedFiles?.length;
|
||||
const modifiedCount = landedFallbackCount && landedFallbackCount > 0
|
||||
? landedFallbackCount
|
||||
: task.modifiedFiles?.length;
|
||||
if (!diffLoading && (modifiedCount ?? 0) > 0) {
|
||||
return (
|
||||
<button
|
||||
type="button"
|
||||
className="card-session-files"
|
||||
onClick={handleOpenFiles}
|
||||
disabled={!onOpenDetailWithTab}
|
||||
title="Captured from worktree during execution; may not match the landed diff. Open the task to view the recorded merge."
|
||||
>
|
||||
<Folder size={12} />
|
||||
<span>{modifiedCount} {modifiedCount === 1 ? "file" : "files"} {landedFallbackCount ? "in merged commit" : "touched during execution"}</span>
|
||||
</button>
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
|
||||
@@ -3135,7 +3135,7 @@ describe("TaskCard", () => {
|
||||
expect(filesChangedButton).toBeNull();
|
||||
});
|
||||
|
||||
it("prefers landedFiles fallback label for done tasks when lineage stats are unavailable", () => {
|
||||
it("prefers landedFiles fallback files-changed label for done tasks when lineage stats are unavailable", () => {
|
||||
const onOpenDetailWithTab = vi.fn();
|
||||
useTaskDiffStatsMock.mockReturnValue({ stats: null, loading: false });
|
||||
|
||||
@@ -3144,7 +3144,6 @@ describe("TaskCard", () => {
|
||||
task={makeTask({
|
||||
column: "done",
|
||||
mergeDetails: { landedFiles: ["a.ts", "b.ts"] },
|
||||
modifiedFiles: ["a.ts", "b.ts", "c.ts", "d.ts", "e.ts", "f.ts"],
|
||||
})}
|
||||
onOpenDetail={noop}
|
||||
addToast={noop}
|
||||
@@ -3152,7 +3151,7 @@ describe("TaskCard", () => {
|
||||
/>,
|
||||
);
|
||||
|
||||
const landedButton = screen.getByRole("button", { name: "2 files in merged commit" });
|
||||
const landedButton = screen.getByRole("button", { name: "2 files changed" });
|
||||
expect(landedButton).toBeDefined();
|
||||
|
||||
fireEvent.click(landedButton);
|
||||
@@ -3160,11 +3159,10 @@ describe("TaskCard", () => {
|
||||
expect(onOpenDetailWithTab.mock.calls[0]?.[1]).toBe("changes");
|
||||
});
|
||||
|
||||
it("shows execution-touched fallback label for done tasks when lineage stats are unavailable", () => {
|
||||
const onOpenDetailWithTab = vi.fn();
|
||||
it("hides the done-task file chip when only execution-touched modifiedFiles exist", () => {
|
||||
useTaskDiffStatsMock.mockReturnValue({ stats: null, loading: false });
|
||||
|
||||
render(
|
||||
const { container } = render(
|
||||
<TaskCard
|
||||
task={makeTask({
|
||||
column: "done",
|
||||
@@ -3172,17 +3170,13 @@ describe("TaskCard", () => {
|
||||
})}
|
||||
onOpenDetail={noop}
|
||||
addToast={noop}
|
||||
onOpenDetailWithTab={onOpenDetailWithTab}
|
||||
onOpenDetailWithTab={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
|
||||
const touchedButton = screen.getByRole("button", { name: "6 files touched during execution" });
|
||||
expect(touchedButton).toBeDefined();
|
||||
expect(screen.queryByText(/\d+ files? changed/i)).toBeNull();
|
||||
|
||||
fireEvent.click(touchedButton);
|
||||
expect(onOpenDetailWithTab).toHaveBeenCalledTimes(1);
|
||||
expect(onOpenDetailWithTab.mock.calls[0]?.[1]).toBe("changes");
|
||||
expect(screen.queryByText(/touched during execution/i)).toBeNull();
|
||||
expect(screen.queryByRole("button", { name: /files changed/i })).toBeNull();
|
||||
expect(container.querySelector(".card-session-files")).toBeNull();
|
||||
});
|
||||
|
||||
it("prefers lineage files-changed stats over stale execution-touched modifiedFiles for done tasks", () => {
|
||||
@@ -3215,7 +3209,26 @@ describe("TaskCard", () => {
|
||||
);
|
||||
|
||||
expect(screen.getByRole("button", { name: "4 files changed" })).toBeDefined();
|
||||
expect(screen.queryByText("10 files touched during execution")).toBeNull();
|
||||
expect(screen.queryByText(/touched during execution/i)).toBeNull();
|
||||
expect(screen.queryByText(/in merged commit/i)).toBeNull();
|
||||
});
|
||||
|
||||
it("uses singular 'file changed' grammar for landedFiles-only done tasks", () => {
|
||||
useTaskDiffStatsMock.mockReturnValue({ stats: null, loading: false });
|
||||
|
||||
render(
|
||||
<TaskCard
|
||||
task={makeTask({
|
||||
column: "done",
|
||||
mergeDetails: { landedFiles: ["a.ts"] },
|
||||
})}
|
||||
onOpenDetail={noop}
|
||||
addToast={noop}
|
||||
onOpenDetailWithTab={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByRole("button", { name: "1 file changed" })).toBeDefined();
|
||||
});
|
||||
|
||||
it("hides done-task file chip when lineage stats are unavailable and no execution fallback exists", () => {
|
||||
|
||||
Reference in New Issue
Block a user