diff --git a/app/src/bindings/ProjectPlan.ts b/app/src/bindings/ProjectPlan.ts index d8d69d8..02c81fa 100644 --- a/app/src/bindings/ProjectPlan.ts +++ b/app/src/bindings/ProjectPlan.ts @@ -38,4 +38,4 @@ agent: AgentRun | null, * file is read back and parsed into [`Self::doc`]. `None` for plans that * were loaded directly from a document rather than produced by an agent. */ -plan_path?: string, }; +plan_path: string | null, }; diff --git a/app/src/bindings/Review.ts b/app/src/bindings/Review.ts index b899aaa..93eacfa 100644 --- a/app/src/bindings/Review.ts +++ b/app/src/bindings/Review.ts @@ -112,12 +112,12 @@ project: ProjectId | null, * Overwritten on each dispatch (see [`DispatchSnapshot`]). `None` before * the first dispatch and for legacy data. */ -dispatch_snapshot?: DispatchSnapshot, +dispatch_snapshot: DispatchSnapshot | null, /** * Rolled-up CI status for this review's PR, when fetched. `None` until a * CI check read populates it, and for legacy data that predates the field. */ -ci_summary?: CiSummary, +ci_summary: CiSummary | null, /** * Advisory findings from the read-only pre-pass reviewer, if it has run. * @@ -135,4 +135,4 @@ conversation: Array, * * `None` before the first pre-pass and for legacy data. */ -last_reviewed_sha?: string, }; +last_reviewed_sha: string | null, }; diff --git a/app/src/components/ReviewCard.tsx b/app/src/components/ReviewCard.tsx index 3390657..e084291 100644 --- a/app/src/components/ReviewCard.tsx +++ b/app/src/components/ReviewCard.tsx @@ -323,7 +323,7 @@ function RiskChips({ review }: { readonly review: Review }) { return ( <> - {ci !== undefined && ci.total > 0 && ( + {ci !== null && ci.total > 0 && ( { expect(isFastLane(root)).toBe(true); }); }); + +describe("null ci_summary (wire shape regression)", () => { + // Serde serializes Option::None as `null` with the key present. The board + // crashed white when attentionRank dereferenced a null summary (the old + // guard only handled `undefined`). These lock the null path. + it("ranks a review whose ci_summary is null without throwing", () => { + const review = makeReview({ ci_summary: null }); + expect(() => attentionRank(review)).not.toThrow(); + // Null CI must rank identically to "no checks loaded" — no adjustment. + const withCi = makeReview({ + id: "rev-ci", + ci_summary: { passed: 1, failed: 0, pending: 0, total: 1 }, + }); + expect(attentionRank(review)).toBeGreaterThan(attentionRank(withCi)); + }); + + it("excludes a null-CI review from the fast lane (absent CI never qualifies)", () => { + const review = makeReview({ ci_summary: null, parents: [] }); + expect(isFastLane(review)).toBe(false); + }); + + it("sorts a mixed board of null and populated summaries without throwing", () => { + const reviews = [ + makeReview({ id: "a", ci_summary: null }), + makeReview({ + id: "b", + ci_summary: { passed: 2, failed: 1, pending: 0, total: 3 }, + }), + makeReview({ id: "c", ci_summary: null, stale: true }), + ]; + expect(() => sortByAttention(reviews)).not.toThrow(); + }); +}); diff --git a/app/src/lib/attention.ts b/app/src/lib/attention.ts index 7e98488..b2977a9 100644 --- a/app/src/lib/attention.ts +++ b/app/src/lib/attention.ts @@ -76,8 +76,10 @@ const SENSITIVE = 0.15; // auth/migrations/config/etc. warrant scrutiny — sink /** The rolled-up CI state for a review, or `"none"` when no checks are loaded. */ function reviewCiState(review: Review): CiState { + // Serde serializes `Option::None` as `null` (the key is always present on + // the wire), so the empty state is `null` — never `undefined`. const ci = review.ci_summary; - return ci === undefined ? "none" : ciState(ci); + return ci === null ? "none" : ciState(ci); } /** Within-bucket adjustment from CI status. */ diff --git a/app/src/test/fixtures.ts b/app/src/test/fixtures.ts index 5b72a71..3a9af52 100644 --- a/app/src/test/fixtures.ts +++ b/app/src/test/fixtures.ts @@ -51,6 +51,12 @@ export function makeReview(overrides: Partial = {}): Review { project: null, review_findings: [], conversation: [], + // Serde emits `Option::None` as `null` with the key present — fixtures + // must mirror the wire shape, not omit the fields (omission hid a + // null-deref crash the type system couldn't see). + dispatch_snapshot: null, + ci_summary: null, + last_reviewed_sha: null, }; return { ...base, ...overrides }; } diff --git a/crates/cockpit-core/src/model.rs b/crates/cockpit-core/src/model.rs index 4d66d7d..cd2c0b0 100644 --- a/crates/cockpit-core/src/model.rs +++ b/crates/cockpit-core/src/model.rs @@ -351,7 +351,6 @@ pub struct ProjectPlan { /// file is read back and parsed into [`Self::doc`]. `None` for plans that /// were loaded directly from a document rather than produced by an agent. #[serde(default)] - #[ts(optional)] pub plan_path: Option, } @@ -524,12 +523,10 @@ pub struct Review { /// Overwritten on each dispatch (see [`DispatchSnapshot`]). `None` before /// the first dispatch and for legacy data. #[serde(default)] - #[ts(optional)] pub dispatch_snapshot: Option, /// Rolled-up CI status for this review's PR, when fetched. `None` until a /// CI check read populates it, and for legacy data that predates the field. #[serde(default)] - #[ts(optional)] pub ci_summary: Option, /// Advisory findings from the read-only pre-pass reviewer, if it has run. /// @@ -545,7 +542,6 @@ pub struct Review { /// /// `None` before the first pre-pass and for legacy data. #[serde(default)] - #[ts(optional)] pub last_reviewed_sha: Option, }