fix: close presentation review findings

This commit is contained in:
lda
2026-07-10 23:18:26 +07:00 Verified
parent 0da27e2b2c
commit 93b6edae4e
12 changed files with 129 additions and 44 deletions
+29
View File
@@ -0,0 +1,29 @@
# Final Fix Report
## Scope
- Reattached persisted, inspectable evidence beneath the typed substrate at wide desktop and retained the 1080px and 640px layout contracts.
- Restored the exact seven defense-topic labels from the closing design.
- Restored the `DefenseDiscussionIndex` component boundary: canonical branches and `openDiscussion` are props; a pure, exhaustively typed projection groups the supplied catalog without copying branch objects.
## TDD Record
- RED: `pnpm --dir web\\apps\\console test -- src/presentation/conclusion/conclusion-model.test.ts src/presentation/conclusion/ConclusionScene.test.tsx src/presentation/discussion/defense-discussion-index.test.ts src/presentation/discussion/DefenseDiscussionIndex.test.tsx src/presentation/presentation-css.test.ts`
- Result: failed as expected, with 7 failures across the missing evidence model/layout contract and missing projection/component API.
- GREEN: `pnpm --dir web\\apps\\console test -- src/presentation/conclusion/conclusion-model.test.ts src/presentation/conclusion/ConclusionScene.test.tsx src/presentation/discussion/defense-discussion-index.test.ts src/presentation/discussion/DefenseDiscussionIndex.test.tsx src/presentation/SceneBody.test.tsx src/presentation/presentation-css.test.ts; pnpm --dir web\\apps\\console typecheck`
- Result: 6 test files passed, 43 tests passed; console typecheck passed.
## Changed Files
- `web/apps/console/src/presentation/conclusion/ConclusionScene.tsx`
- `web/apps/console/src/presentation/conclusion/conclusion-model.ts`
- `web/apps/console/src/presentation/presentation.css`
- `web/apps/console/src/presentation/discussion/DefenseDiscussionIndex.tsx`
- `web/apps/console/src/presentation/discussion/defense-discussion-index.ts`
- `web/apps/console/src/presentation/SceneBody.tsx`
- Focused conclusion, discussion, SceneBody, and presentation CSS tests.
## Deviations And Concerns
- Deviations: none.
- Concerns: no manual browser screenshot pass was run in this fix wave; the requested responsive behavior is covered by CSS contract tests at 1080px and 640px.
@@ -328,7 +328,7 @@ export const SceneBody = ({ location, demo, selectedNodeId, selectNode, openEvid
return <EvaluationEvidenceScene scene={scene} beat={beat} />; return <EvaluationEvidenceScene scene={scene} beat={beat} />;
case "conclusion": case "conclusion":
return beat.id === "questions" return beat.id === "questions"
? <DefenseDiscussionIndex openDiscussion={openDiscussion} /> ? <DefenseDiscussionIndex discussionBranches={discussionBranches} openDiscussion={openDiscussion} />
: <ConclusionScene scene={scene} beat={beat} />; : <ConclusionScene scene={scene} beat={beat} />;
default: default:
return assertNever(scene.view); return assertNever(scene.view);
@@ -77,17 +77,17 @@ describe("ConclusionScene", () => {
expect(screen.getByRole("list", { name: "future work layers" })).toHaveAttribute("data-state", "receded"); expect(screen.getByRole("list", { name: "future work layers" })).toHaveAttribute("data-state", "receded");
}); });
it("keeps evidence as a separately identified node after the runtime", () => { it("attaches evidence vertically beneath the typed substrate rather than extending the contribution line", () => {
render(<ConclusionScene scene={scene} beat={beat("future")} />); render(<ConclusionScene scene={scene} beat={beat("future")} />);
const nodes = [...screen.getByLabelText("contribution flow").children]; const executionNodes = [...screen.getByLabelText("contribution flow").children]
expect(nodes.map((node) => node.getAttribute("data-node-id"))).toEqual([ .filter((node) => node.getAttribute("data-evidence-attachment") !== "vertical");
expect(executionNodes.map((node) => node.getAttribute("data-node-id"))).toEqual([
"planner", "planner",
"substrate", "substrate",
"runtime", "runtime",
"evidence",
]); ]);
expect(nodes[2]).toHaveAttribute("data-node-id", "runtime"); expect(screen.getByText("Persisted, inspectable evidence").closest("[data-node-id='evidence']"))
expect(nodes[3]).toHaveAttribute("data-node-id", "evidence"); .toHaveAttribute("data-evidence-attachment", "vertical");
}); });
it("marks every future-work icon neutral while substrate stays the sole emphasis", () => { it("marks every future-work icon neutral while substrate stays the sole emphasis", () => {
@@ -4,6 +4,7 @@ import { StageCaption } from "../StageCaption.js";
import type { SceneBeatDefinition, SceneDefinition } from "../storyboard.js"; import type { SceneBeatDefinition, SceneDefinition } from "../storyboard.js";
import { import {
contributionNodes, contributionNodes,
evidenceNode,
futureWorkBranches, futureWorkBranches,
isConclusionBeatId, isConclusionBeatId,
nonClaims, nonClaims,
@@ -43,9 +44,17 @@ export const ConclusionScene: FC<ConclusionSceneProps> = ({ scene, beat }) => {
> >
<span className="conclusion-map__node-index">0{index + 1}</span> <span className="conclusion-map__node-index">0{index + 1}</span>
<strong>{node.label}</strong> <strong>{node.label}</strong>
{node.id === "evidence" && <small>saved traces and receipts</small>}
</div> </div>
))} ))}
<div
className={`conclusion-map__node conclusion-map__node--${evidenceNode.id}`}
data-evidence-attachment="vertical"
data-node-id={evidenceNode.id}
>
<span className="conclusion-map__node-index">04</span>
<strong>{evidenceNode.label}</strong>
<small>saved traces and receipts</small>
</div>
</div> </div>
<ul <ul
@@ -1,5 +1,5 @@
import { describe, expect, it } from "vitest"; import { describe, expect, it } from "vitest";
import { contributionNodes, futureWorkBranches, isConclusionBeatId, nonClaims } from "./conclusion-model.js"; import { contributionNodes, evidenceNode, futureWorkBranches, isConclusionBeatId, nonClaims } from "./conclusion-model.js";
describe("conclusion model", () => { describe("conclusion model", () => {
it("defines the stable contribution boundary", () => { it("defines the stable contribution boundary", () => {
@@ -7,8 +7,8 @@ describe("conclusion model", () => {
{ id: "planner", label: "External planner" }, { id: "planner", label: "External planner" },
{ id: "substrate", label: "Typed workflow substrate" }, { id: "substrate", label: "Typed workflow substrate" },
{ id: "runtime", label: "Deterministic runtime" }, { id: "runtime", label: "Deterministic runtime" },
{ id: "evidence", label: "Persisted, inspectable evidence" },
]); ]);
expect(evidenceNode).toEqual({ id: "evidence", label: "Persisted, inspectable evidence" });
}); });
it("keeps the closing claims bounded", () => { it("keeps the closing claims bounded", () => {
@@ -5,9 +5,11 @@ export const contributionNodes = [
{ id: "planner", label: "External planner" }, { id: "planner", label: "External planner" },
{ id: "substrate", label: "Typed workflow substrate" }, { id: "substrate", label: "Typed workflow substrate" },
{ id: "runtime", label: "Deterministic runtime" }, { id: "runtime", label: "Deterministic runtime" },
{ id: "evidence", label: "Persisted, inspectable evidence" },
] as const; ] as const;
// Evidence is attached to the substrate, not another step in the execution line.
export const evidenceNode = { id: "evidence", label: "Persisted, inspectable evidence" } as const;
export const nonClaims = ["Not a production sandbox", "Not a scheduler", "Not a broad agent benchmark"] as const; export const nonClaims = ["Not a production sandbox", "Not a scheduler", "Not a broad agent benchmark"] as const;
export const futureWorkBranches = [ export const futureWorkBranches = [
@@ -1,18 +1,18 @@
import { cleanup, fireEvent, render, screen, within } from "@testing-library/react"; import { cleanup, fireEvent, render, screen, within } from "@testing-library/react";
import { afterEach, describe, expect, it, vi } from "vitest"; import { afterEach, describe, expect, it, vi } from "vitest";
import { discussionBranches } from "../storyboard.js"; import { discussionBranches } from "../storyboard.js";
import { defenseDiscussionGroups } from "./defense-discussion-index.js"; import { projectDefenseDiscussionGroups } from "./defense-discussion-index.js";
import { DefenseDiscussionIndex } from "./DefenseDiscussionIndex.js"; import { DefenseDiscussionIndex } from "./DefenseDiscussionIndex.js";
describe("DefenseDiscussionIndex", () => { describe("DefenseDiscussionIndex", () => {
afterEach(cleanup); afterEach(cleanup);
it("renders seven labelled topic sections and every canonical branch title", () => { it("renders seven labelled topic sections and every canonical branch title", () => {
render(<DefenseDiscussionIndex openDiscussion={vi.fn()} />); render(<DefenseDiscussionIndex discussionBranches={discussionBranches} openDiscussion={vi.fn()} />);
const nav = screen.getByRole("navigation", { name: "defense discussion index" }); const nav = screen.getByRole("navigation", { name: "defense discussion index" });
expect(within(nav).getAllByRole("heading", { level: 2 })).toHaveLength(7); expect(within(nav).getAllByRole("heading", { level: 2 })).toHaveLength(7);
for (const group of defenseDiscussionGroups) { for (const group of projectDefenseDiscussionGroups(discussionBranches)) {
const heading = within(nav).getByRole("heading", { name: group.label, level: 2 }); const heading = within(nav).getByRole("heading", { name: group.label, level: 2 });
expect(heading.querySelector("svg")).not.toBeNull(); expect(heading.querySelector("svg")).not.toBeNull();
} }
@@ -23,7 +23,7 @@ describe("DefenseDiscussionIndex", () => {
it("opens the canonical branch selected from the index", () => { it("opens the canonical branch selected from the index", () => {
const openDiscussion = vi.fn(); const openDiscussion = vi.fn();
render(<DefenseDiscussionIndex openDiscussion={openDiscussion} />); render(<DefenseDiscussionIndex discussionBranches={discussionBranches} openDiscussion={openDiscussion} />);
fireEvent.click(screen.getByRole("button", { name: "Live demo reliability" })); fireEvent.click(screen.getByRole("button", { name: "Live demo reliability" }));
@@ -1,6 +1,6 @@
import type { FC } from "react"; import type { FC } from "react";
import { BadgeHelp, Boxes, ChartNoAxesCombined, FileCode2, Map, PlaySquare, Rocket } from "lucide-react"; import { BadgeHelp, Boxes, ChartNoAxesCombined, FileCode2, Map, PlaySquare, Rocket } from "lucide-react";
import { defenseDiscussionGroups, type DefenseDiscussionTopicId } from "./defense-discussion-index.js"; import { projectDefenseDiscussionGroups, type CanonicalDiscussionBranchDefinition, type DefenseDiscussionTopicId } from "./defense-discussion-index.js";
const topicIcons = { const topicIcons = {
contribution: BadgeHelp, contribution: BadgeHelp,
@@ -12,11 +12,18 @@ const topicIcons = {
production: Rocket, production: Rocket,
} as const satisfies Record<DefenseDiscussionTopicId, typeof BadgeHelp>; } as const satisfies Record<DefenseDiscussionTopicId, typeof BadgeHelp>;
export const DefenseDiscussionIndex: FC<{ readonly openDiscussion: (branchId: string) => void }> = ({ export const DefenseDiscussionIndex: FC<{
readonly discussionBranches: readonly CanonicalDiscussionBranchDefinition[];
readonly openDiscussion: (branchId: string) => void;
}> = ({
discussionBranches,
openDiscussion, openDiscussion,
}) => ( }) => {
const groups = projectDefenseDiscussionGroups(discussionBranches);
return (
<nav className="defense-discussion-index" aria-label="defense discussion index"> <nav className="defense-discussion-index" aria-label="defense discussion index">
{defenseDiscussionGroups.map((group) => { {groups.map((group) => {
const Icon = topicIcons[group.id]; const Icon = topicIcons[group.id];
return ( return (
<section className="defense-discussion-index__group" key={group.id}> <section className="defense-discussion-index__group" key={group.id}>
@@ -37,4 +44,5 @@ export const DefenseDiscussionIndex: FC<{ readonly openDiscussion: (branchId: st
); );
})} })}
</nav> </nav>
); );
};
@@ -1,10 +1,11 @@
import { describe, expect, it } from "vitest"; import { describe, expect, it } from "vitest";
import { discussionBranches } from "../storyboard.js"; import { discussionBranches } from "../storyboard.js";
import { defenseDiscussionGroups, discussionTopicByBranchId } from "./defense-discussion-index.js"; import { discussionTopicByBranchId, projectDefenseDiscussionGroups } from "./defense-discussion-index.js";
describe("defense discussion index", () => { describe("defense discussion index", () => {
it("exhaustively projects every canonical discussion branch exactly once", () => { it("exhaustively projects every canonical discussion branch exactly once", () => {
const indexedIds = defenseDiscussionGroups.flatMap((group) => group.branches.map((branch) => branch.id)); const groups = projectDefenseDiscussionGroups(discussionBranches);
const indexedIds = groups.flatMap((group) => group.branches.map((branch) => branch.id));
expect(indexedIds).toHaveLength(discussionBranches.length); expect(indexedIds).toHaveLength(discussionBranches.length);
expect(new Set(indexedIds)).toEqual(new Set(discussionBranches.map((branch) => branch.id))); expect(new Set(indexedIds)).toEqual(new Set(discussionBranches.map((branch) => branch.id)));
@@ -14,9 +15,22 @@ describe("defense discussion index", () => {
}); });
it("derives indexed branch objects from the canonical definitions", () => { it("derives indexed branch objects from the canonical definitions", () => {
const groups = projectDefenseDiscussionGroups(discussionBranches);
for (const branch of discussionBranches) { for (const branch of discussionBranches) {
const indexed = defenseDiscussionGroups.flatMap((group) => group.branches).find(({ id }) => id === branch.id); const indexed = groups.flatMap((group) => group.branches).find(({ id }) => id === branch.id);
expect(indexed).toBe(branch); expect(indexed).toBe(branch);
} }
}); });
it("uses the scoped defense-topic labels from the closing design", () => {
expect(projectDefenseDiscussionGroups(discussionBranches).map((group) => group.label)).toEqual([
"Thesis contribution",
"Positioning and related systems",
"Runtime and lifecycle",
"Authoring and validation",
"Demo integrity",
"Evaluation",
"Production readiness and future work",
]);
});
}); });
@@ -15,6 +15,10 @@ export type DefenseDiscussionGroup = {
readonly branches: readonly DiscussionBranchDefinition[]; readonly branches: readonly DiscussionBranchDefinition[];
}; };
export type CanonicalDiscussionBranchDefinition = DiscussionBranchDefinition & {
readonly id: DiscussionBranchId;
};
// This explicit record is intentionally exhaustive: adding a Q&A branch must // This explicit record is intentionally exhaustive: adding a Q&A branch must
// also place it in the end-of-defense index instead of silently hiding it. // also place it in the end-of-defense index instead of silently hiding it.
export const discussionTopicByBranchId: Record<DiscussionBranchId, DefenseDiscussionTopicId> = { export const discussionTopicByBranchId: Record<DiscussionBranchId, DefenseDiscussionTopicId> = {
@@ -43,16 +47,19 @@ export const discussionTopicByBranchId: Record<DiscussionBranchId, DefenseDiscus
}; };
const discussionTopicGroups = [ const discussionTopicGroups = [
{ id: "contribution", label: "Contribution" }, { id: "contribution", label: "Thesis contribution" },
{ id: "positioning", label: "Positioning" }, { id: "positioning", label: "Positioning and related systems" },
{ id: "runtime", label: "Runtime" }, { id: "runtime", label: "Runtime and lifecycle" },
{ id: "authoring", label: "Authoring" }, { id: "authoring", label: "Authoring and validation" },
{ id: "demo", label: "Demo" }, { id: "demo", label: "Demo integrity" },
{ id: "evaluation", label: "Evaluation" }, { id: "evaluation", label: "Evaluation" },
{ id: "production", label: "Production" }, { id: "production", label: "Production readiness and future work" },
] as const satisfies readonly { id: DefenseDiscussionTopicId; label: string }[]; ] as const satisfies readonly { id: DefenseDiscussionTopicId; label: string }[];
export const defenseDiscussionGroups: readonly DefenseDiscussionGroup[] = discussionTopicGroups.map((topic) => ({ /** Projects canonical branches without copying their titles or answer content. */
export const projectDefenseDiscussionGroups = (branches: readonly CanonicalDiscussionBranchDefinition[]): readonly DefenseDiscussionGroup[] => discussionTopicGroups.map((topic) => ({
...topic, ...topic,
branches: discussionBranches.filter((branch) => discussionTopicByBranchId[branch.id] === topic.id), branches: branches.filter((branch) => discussionTopicByBranchId[branch.id] === topic.id),
})); }));
export const defenseDiscussionGroups = projectDefenseDiscussionGroups(discussionBranches);
@@ -31,12 +31,14 @@ describe("presentation.css", () => {
expect(boardBlock).toContain("flex-shrink: 0"); expect(boardBlock).toContain("flex-shrink: 0");
}); });
it("places the conclusion evidence beneath substrate at the 1080px breakpoint", () => { it("keeps evidence vertically attached beneath substrate from wide desktop through the 1080px breakpoint", () => {
expect(css).toMatch(/\.conclusion-map__flow\s*\{\s*display: grid;\s*grid-template-columns: repeat\(3, minmax\(0, 1fr\)\);\s*grid-template-rows: auto auto;/);
expect(css).toMatch(/\.conclusion-map__node--planner\s*\{\s*grid-column: 1;\s*grid-row: 1;/); expect(css).toMatch(/\.conclusion-map__node--planner\s*\{\s*grid-column: 1;\s*grid-row: 1;/);
expect(css).toMatch(/\.conclusion-map__node--substrate\s*\{\s*grid-column: 2;\s*grid-row: 1;/); expect(css).toMatch(/\.conclusion-map__node--substrate\s*\{\s*grid-column: 2;\s*grid-row: 1;/);
expect(css).toMatch(/\.conclusion-map__node--runtime\s*\{\s*grid-column: 3;\s*grid-row: 1;/); expect(css).toMatch(/\.conclusion-map__node--runtime\s*\{\s*grid-column: 3;\s*grid-row: 1;/);
expect(css).toMatch(/\.conclusion-map__node--evidence\s*\{\s*grid-column: 2;\s*grid-row: 2;/); expect(css).toMatch(/\.conclusion-map__node--evidence\s*\{\s*grid-column: 2;\s*grid-row: 2;/);
expect(css).not.toMatch(/\.conclusion-map__node--runtime::after/); expect(css).not.toMatch(/\.conclusion-map__node--runtime::after/);
expect(css).toMatch(/\.conclusion-map__node--evidence::before\s*\{[\s\S]*?content: "↓";/);
}); });
it("uses a light foreground against the dark conclusion map", () => { it("uses a light foreground against the dark conclusion map", () => {
@@ -275,7 +275,9 @@
.conclusion-map__flow { .conclusion-map__flow {
display: grid; display: grid;
grid-template-columns: repeat(4, minmax(0, 1fr)); grid-template-columns: repeat(3, minmax(0, 1fr));
grid-template-rows: auto auto;
gap: 0.8rem 1.7rem;
align-items: center; align-items: center;
} }
@@ -305,6 +307,25 @@
text-align: right; text-align: right;
} }
/* The evidence artifact descends from the substrate rather than execution. */
.conclusion-map__node--evidence::before {
position: absolute;
top: -0.85rem;
left: 50%;
z-index: 1;
height: 0.85rem;
border-left: 1px solid var(--text-muted);
content: "↓";
color: var(--text-muted);
font-size: 1rem;
line-height: 0;
}
.conclusion-map__node--planner { grid-column: 1; grid-row: 1; }
.conclusion-map__node--substrate { grid-column: 2; grid-row: 1; }
.conclusion-map__node--runtime { grid-column: 3; grid-row: 1; }
.conclusion-map__node--evidence { grid-column: 2; grid-row: 2; }
.conclusion-map__node[data-emphasis="substrate"] { .conclusion-map__node[data-emphasis="substrate"] {
border-color: var(--accent-cyan); border-color: var(--accent-cyan);
box-shadow: inset 0 0 0 1px oklch(0.72 0.17 195 / 20%); box-shadow: inset 0 0 0 1px oklch(0.72 0.17 195 / 20%);
@@ -382,15 +403,8 @@
@media (max-width: 1080px) { @media (max-width: 1080px) {
.conclusion-map__flow { .conclusion-map__flow {
grid-template-columns: repeat(3, minmax(0, 1fr)); gap: 0.75rem 1.25rem;
grid-template-rows: auto auto;
gap: 0.8rem 1.7rem;
} }
.conclusion-map__node--planner { grid-column: 1; grid-row: 1; }
.conclusion-map__node--substrate { grid-column: 2; grid-row: 1; }
.conclusion-map__node--runtime { grid-column: 3; grid-row: 1; }
.conclusion-map__node--evidence { grid-column: 2; grid-row: 2; }
} }
@media (max-width: 640px) { @media (max-width: 640px) {