Skip to content

feat(change-set): one delivery overview per issue, from the readiness collection - #218

Open
SoloJiang wants to merge 9 commits into
mainfrom
claude/issue-175-change-set
Open

feat(change-set): one delivery overview per issue, from the readiness collection#218
SoloJiang wants to merge 9 commits into
mainfrom
claude/issue-175-change-set

Conversation

@SoloJiang

Copy link
Copy Markdown
Owner

Closes #175.

An issue's state is spread across its Directions, worktrees, Evidence rows and Sessions, so answering what will be written, why those repos, in what order, how much is trustworthy, what is left means opening a Session and assembling it by hand. This assembles it once.

One collection, two projections

readiness already computes every one of those facts on its way to a verdict — including the Git signature probe of each lane's registered worktrees — and then discards whatever the verdict did not consume. The Change Set takes them from that same collection rather than re-deriving them: two derivations are two answers that can disagree, which is the drift readiness exists to prevent. The verdict shown is the verdict issue_readiness would give, by construction rather than by agreement.

Concretely, collect_with_check_execution splits into collect_facts_and_readiness, which returns (IssueReadinessDto, Vec<LaneFacts>), and LaneFacts gains checkouts: Option<Vec<LaneCheckout>> carrying the probe samples that reconciliation_for reduced to a single word.

None and Some([]) are deliberately distinct: None means this collection never probed (the same paths where reconciliation is Unknown for that reason), Some([]) means it probed and no worktree is registered yet. reconciliation reads Unknown for both, so without the distinction a reader cannot tell we did not look from there is nothing there yet — and those need different next actions. The distinction survives to the screen.

What the panel shows

A third issue tab beside the lead chat and the board. Each active lane is one row: the repo it writes and why, the declared base/branch beside what the probe observed, the lane's own readiness chip, checks/upstream/reconciliation, evidence trust, and its PRs. Lanes are grouped into dependency waves — step 1 is everything waiting on nothing, step N everything whose upstreams landed earlier.

An edge pointing at a lane the verdict excluded is ignored rather than treated as unsatisfiable, and a dependency cycle is emitted as one honest block rather than dropped or given an order the edges do not support.

Three defects of one shape, caught during the work

Each is an identity collapsed onto something that is not an identity.

The verdict join. Virtual lanes — the unbound-PR row, the issue-wide ask row, every unmaterialized proposed lane — all carry direction_id == 0. issue_readiness builds its lane list by filtering the same facts in order, so its rows are an ordered subsequence and an ordered cursor walk pairs them exactly; a direction_id map would hand several distinct lanes one shared verdict.

The wave layout reintroduced that same flaw in the frontend, keying pending lanes by direction_id, so an issue with more than one virtual lane kept only the last and silently dropped the rest while the header still counted them. Lanes are now tracked by position throughout.

The evidence join had it a third time, one field later: bucketing by direction_id handed every virtual lane the issue's own evidence as if it had produced it. Issue-level rows are now reported separately as issue_evidence.

Two more from review

newest_observed_at took the first row of an id-ordered scan, but append_evidence refreshes observed_at in place on a dedupe hit without moving the id — so a re-confirmed fact was reported as older than it is. It now compares timestamps, and an unparsable one never displaces a comparable one.

The evidence scan bound is per issue while the summaries are per lane, so a busy lane can push a quiet one's rows past the cut and leave it reading as having no evidence at all. Hitting the bound is now reported (evidence_scan_truncated) instead of being passed off as a complete count.

A design mistake worth recording

Making issue_change_set a separate command — to keep the polled readiness path light — backfired. The board then ran two collections against one thread, their Git probes contended, and a probe that lost its budget reported unknown: the panel claimed "checkout could not be read" while the chip beside it, fed by the other collection, said the lane was fine. That is exactly the disagreement this module's contract claims is impossible; the contract held per collection and was broken by running two.

Measured on the running app: 0 of 12 lane probes failed with the reads sequential, 1 of 12 with a Change Set read racing a readiness poll. The board now issues one command per refresh — the richer one when the tab is open — and both surfaces read that single result. After the fix: 7 consecutive tab round-trips, both lanes checkout matches, zero spurious unknowns.

Constraints

All multi-way state is derived once as a discriminated value and mapped exhaustively; nothing re-tests booleans per call site. The checkout state comes from the backend reconciliation, never from a frontend re-comparison — and a blank declared branch is not something a checkout can differ from, since reconciliation_for declines to judge that case at all. The lane status goes through one LaneStatusView mapped to i18n keys, with an unrecognized token rendering as "status unknown" rather than raw: it previously leaked the store's queued token untranslated into the Chinese UI.

Verification

  • cargo test --lib change_set — 13 passed
  • cargo test --lib readiness — 80 passed
  • cargo test --test readiness — 49 passed; m2_git, m2_worktree, lead_repo_state, delete_late_writes, worktree_delete also green
  • pnpm build, pnpm test — 208 passed, npx tsc --noEmit clean, git diff --check clean
  • Driven on the running Tauri/WebView surface under Xvfb with a real two-repo fixture (api-serviceweb-client join dependency), in both locales

Three --lib tests fail in this sandbox and are unrelated: checkpoint::mid_restore_failure_rolls_everything_back needs a chmod-undeletable file, which is impossible as uid 0 (verified: root deletes through a read-only directory here), and two proc_registry process-group reaping tests. Neither module references readiness outside doc comments.

Not done here

evidence_scan_truncated is reported but no UI reads it yet — surfacing it is a small follow-up.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Xz1Ts3AK1uNbeWZT6YFbzz


Generated by Claude Code

claude added 5 commits August 11, 2026 02:11
…ollection

The state of an issue is spread across its Directions, worktrees, Evidence
rows and Sessions, so answering "what will be written, why those repos, in
what order, how much is trustworthy, what is left" means opening a Session
and assembling it by hand.

`readiness` already computes every one of those facts on its way to a verdict
— including the Git signature probe of each lane's registered worktrees — and
then discards whatever the verdict did not consume. This adds a projection
that takes them from that SAME collection instead of re-deriving them: two
derivations are two answers that can disagree, which is the drift `readiness`
exists to prevent, and a second probe would pay for the same `git` calls twice
on the path that paints the first screen.

- readiness: carry the probe samples on `LaneFacts` as `checkouts`, an
  `Option<Vec<LaneCheckout>>`. `None` = this collection never probed (the
  same paths where `reconciliation` is Unknown for that reason); `Some([])` =
  probed, nothing registered yet. Collapsing those two would leave a reader
  unable to tell "we did not look" from "there is nothing there".
- change_set: join the discarded facts with the rows that say WHERE each lane
  writes — repo, selection reason, declared base/branch, dependency order,
  evidence trust — and reuse the verdict's own per-lane answer verbatim.
- Pair verdicts to facts by an ordered cursor walk, not a `direction_id` map:
  the unbound-PR row, the issue-wide ask row and every unmaterialized proposed
  lane all carry `direction_id == 0`, so keying on it would hand several
  distinct lanes one shared verdict.
- Judge revision-anchored Evidence against the head SHAs the collection
  already sampled, so the summary reports real freshness instead of a blanket
  `unknown`; a lane that was never probed still fails closed.
- Expose it as `issue_change_set` rather than widening `issue_readiness`,
  which the board surfaces poll per thread.

Verified: cargo test --lib change_set (10 passed), cargo test --lib readiness
(80 passed), git diff --check.

Refs #175

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xz1Ts3AK1uNbeWZT6YFbzz
The backend overview needed a screen. This adds a third issue tab beside the
lead chat and the board, showing every active lane as one row: the repo it
writes and why, the declared base/branch beside what the probe observed, the
lane's own readiness chip, its checks/upstream/reconciliation state, its
evidence trust, and its PRs.

- Lanes are grouped into dependency waves — step 1 is everything waiting on
  nothing, step N everything whose upstreams landed earlier — so "in what
  order" is readable without reconstructing it lane by lane. An edge pointing
  at a lane the verdict excluded is ignored rather than treated as
  unsatisfiable, and a dependency cycle is emitted as one honest block instead
  of being dropped or given an order the edges do not support.
- All multi-way state is derived ONCE in `changeSetView.ts` as a discriminated
  value and mapped exhaustively; nothing re-tests booleans per call site. In
  particular `not_probed` and `none_registered` stay separate all the way to
  the screen, and the checkout state comes from the backend `reconciliation`
  rather than from a frontend re-comparison of the branches.
- The declared-branch highlight is explicitly a display pointer at the row
  worth reading, not a second verdict; an unsampled row is neither agreeing
  nor disagreeing and is left unmarked.
- `repo_id` joins `repo_name` on the lane DTO so a row can open its Session
  directly: two repos may share a display name and only the id is unique.
- The panel re-reads on the board's existing readiness refresh key rather than
  inventing a second invalidation rule, and follows the house contract that a
  refresh never presents a prior verdict as current evidence.

Verified: pnpm build, pnpm test (201 passed), npx tsc --noEmit,
git diff --check.

Refs #175

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xz1Ts3AK1uNbeWZT6YFbzz
…llapse

`laneWaves` tracked pending lanes in a Map keyed by `direction_id`. The
unbound-PR row, the issue-wide ask row and every proposed lane not yet
materialized ALL carry `direction_id === 0`, so an issue with more than one of
them kept only the last: the others were silently dropped from the panel while
the header still counted them. The React keys had the same flaw one layer up.

This is the defect the backend join was already written to avoid; the frontend
layout had reintroduced it. Lanes are now tracked by POSITION throughout, with
`direction_id` used only to resolve dependency edges — where `0` names no lane
and the backend already drops those edges.

Tests: three new cases covering several lanes sharing id 0, virtual lanes mixed
with real dependent lanes, and a cycle among them; every lane must appear
exactly once.

Verified: pnpm test (15 in changeSetView, 204 total), npx tsc --noEmit.

Refs #175

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xz1Ts3AK1uNbeWZT6YFbzz
…ontend with the chip

Opening the Change Set tab made the board issue TWO collections for the same
thread: its existing `issue_readiness` poll and the panel's own
`issue_change_set`. Both run the same collection and the same bounded Git
signature probe of every registered worktree, so they contend — and a probe
that loses its budget reports `unknown`.

The user-visible result was the panel claiming "checkout could not be read"
and downgrading a lane's evidence to unknown while the chip beside it, from
the other collection, said the lane was fine. That is exactly the disagreement
`change_set`'s "one collection, two projections" contract exists to rule out;
the contract held per collection and I had broken it by running two.

Measured on the running app before the fix: 0 of 12 lane probes failed with
the reads sequential, 1 of 12 with a Change Set read racing a readiness poll.
After: 7 consecutive tab round-trips, both lanes "checkout matches", zero
spurious unknowns.

- New `issueDelivery.ts`: ONE stored delivery read, produced by whichever
  command the open tab called for, feeding both the chip and the panel.
- The open tab joins the refresh key, so switching to the Change Set re-reads
  and switching away stops paying for the heavier command.
- `ChangeSetPanel` no longer fetches — it is now a pure function of that read,
  the same split `ReadinessChip` and `EvidenceBody` already use.
- A ready read that carries no change set (the readiness command answered)
  renders as loading, never as empty: "this issue writes nothing" is a
  different and wrong claim.

Verified: pnpm build, pnpm test (206 passed), npx tsc --noEmit,
git diff --check, and on the running Tauri/WebView surface in both locales.

Refs #175

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xz1Ts3AK1uNbeWZT6YFbzz
…tatus token

Four defects an adversarial review surfaced, all of them the same shape: a
value presented as an answer it had not earned.

- `evidence_by_direction` bucketed by `direction_id`, so every virtual lane —
  which all carry `0` — was handed the issue's own evidence as if it had
  produced it. That is the collision the verdict join was written to avoid,
  reintroduced one field later. Issue-level rows are now kept apart and
  reported as `issue_evidence`; a virtual lane reports none, because it has
  none.
- `newest_observed_at` took the first row of an id-ordered scan, but
  `append_evidence` refreshes `observed_at` in place on a dedupe hit without
  moving the id. Id order is not observation order, so a re-confirmed fact was
  reported as older than it is. It now compares the timestamps, and an
  unparsable one never displaces a comparable one.
- The evidence scan bound is per ISSUE while the summaries are per LANE, so a
  busy lane can push a quiet one's rows past the cut and leave it reading as
  having no evidence at all. Hitting the bound is now reported
  (`evidence_scan_truncated`) instead of being passed off as a complete count;
  the doc comment claiming truncation only drops the least interesting rows
  was simply wrong.
- The lane row printed the store's raw `direction_status` token, putting an
  untranslated backend string on screen — visible as "queued" in the Chinese
  UI. It now goes through one discriminated `LaneStatusView` mapped
  exhaustively to i18n keys, with an unrecognized token rendering as "status
  unknown" rather than raw.

Also: a blank declared branch is no longer treated as something a checkout can
differ from. `reconciliation_for` returns Unknown for an empty declared branch
rather than judging it, so highlighting a row as "differs" was the frontend
inventing a comparison the backend had declined to make.

Verified: cargo test --lib change_set (13 passed) and readiness (80 passed),
pnpm build, pnpm test (208 passed), git diff --check, and on the running
Tauri/WebView surface in both locales — the status now reads "排队中" where it
previously showed "queued".

Refs #175

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xz1Ts3AK1uNbeWZT6YFbzz

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 097a4932e7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/board/changeSetView.ts Outdated
Comment on lines +101 to +104
if (ready.length === 0) {
// Every lane left is in or behind a cycle. Emit them as one wave.
waves.push({ lanes: [...pending].map((position) => lanes[position]) });
break;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Separate cycle members from lanes blocked behind the cycle

When a dependency cycle has downstream consumers (for example, A depends on B, B depends on A, and C depends on A), this branch emits A, B, and C in the same wave. The panel defines a wave as lanes that can proceed together, so C is incorrectly presented as parallel with its still-blocked upstream; moreover, if an independent wave preceded these lanes, the ordinary wave header falsely says the whole block waits on that preceding step. Represent the cycle as a distinct state and continue ordering its downstream lanes separately rather than dumping every pending lane into one normal wave.

AGENTS.md reference: AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

Comment thread src/lib/types.ts
Comment on lines +414 to +419
export interface IssueChangeSet {
readiness: IssueReadiness;
reasons: ReadinessReason[];
active_lane_count: number;
lanes: ChangeSetLane[];
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Surface truncated evidence scans before claiming no evidence

When an issue has more than 200 evidence rows, the backend intentionally returns evidence_scan_truncated: true because an older lane can receive a zero summary even though it has evidence beyond the scan limit. This frontend DTO omits that flag, so ChangeSetPanel unconditionally renders “no evidence recorded” for such a lane, turning an incomplete scan into a false assertion. Preserve the truncation field and render an incomplete/unknown indication instead of the empty-state label.

Useful? React with 👍 / 👎.

Comment thread src/board/ChangeSetPanel.tsx Outdated
Comment on lines +160 to +162
<ReadinessChip state={{ kind: "ready", dto: lane }} className="max-w-[20rem]" />
<span className="ml-auto shrink-0 text-[10.5px] text-ink-faint">
{t(STATUS_KEYS[laneStatusView(lane.direction_status)])}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Distinguish virtual lanes before rendering lifecycle status

Whenever the collector adds an unbound-PR row, an issue-wide ask, or an unmaterialized proposed lane, readiness assigns the synthetic direction_status value working only to make its verdict fail closed; it is not a real lifecycle state. Rendering that token through STATUS_KEYS labels every such virtual row as “building,” even though an issue ask or unbound PR has no worker building anything. Derive a status view that accounts for direction_id === 0 instead of presenting the readiness sentinel as stored lane status.

AGENTS.md reference: AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

Comment thread src/board/ChangeSetPanel.tsx Outdated
Comment on lines +203 to +207
{lane.pull_requests.length > 0 && (
<span className="inline-flex items-center gap-1">
<GitPullRequest size={11} aria-hidden="true" />
{t("changeSet.pullRequests", { count: lane.pull_requests.length })}
</span>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Show which pull request is blocking the lane

When a lane has multiple tracked PRs, especially when only one has failing CI, requested changes, unresolved threads, or a conflict, this renders only “N pull requests.” The serialized facts contain the axes but omit the persisted PR number, URL, title, and repository identity, and none of the axes are rendered, so the user cannot determine which PR produced the lane verdict or what action to take without leaving the overview. Project a display identity from the stored PR row and render each PR's relevant host state rather than only its count.

Useful? React with 👍 / 👎.

Comment on lines +148 to +150
// A virtual lane (unbound PR row, issue-wide ask) has no direction to open,
// and a lane whose write repo was never resolved has nothing to open it in.
const openable = lane.direction_id !== 0 && lane.repo_id !== 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Disable the lane opener when its repository is unresolved

When a direction retains a dangling repo_id, the backend explicitly tolerates that state by returning an empty repo_name while preserving the nonzero ID. This predicate therefore still renders the opener, and clicking it calls viewDirection with a repository that does not exist, leading to an unusable session view instead of keeping the degraded Change Set visible. Treat the failed repository resolution as non-openable, for example by returning repo_id: 0 or checking the resolved repository state here.

Useful? React with 👍 / 👎.

…support

All from Codex on #218; all confirmed against the code.

**A cycle is not a step.** The layout dumped every remaining lane into one wave
when it got stuck, so a lane merely blocked BEHIND a deadlock was presented as
running in parallel with its own blocked upstream — and the ordinary "waits on
the step above" header described an order that does not exist. Only lanes
actually ON the cycle are now emitted, as their own `cycle` kind with its own
header; downstream consumers keep their real position in later waves.

**A readiness sentinel is not a lifecycle.** `virtual_lane_facts` sets
`direction_status: "working"` purely so a virtual lane's verdict fails closed.
Rendering that token labelled every unbound-PR row and issue-wide ask as
"building", asserting work nobody is doing. Lanes now carry an explicit
`materialized` flag — reported rather than inferred from `direction_id == 0`,
since a proposed lane whose direction was deleted keeps a non-zero id — and an
unmaterialized lane renders as "not started".

**An id whose row is gone is not an openable id.** A direction can retain a
dangling `repo_id`; the backend tolerated it by returning an empty name while
still handing out the id, so the opener stayed enabled and would navigate to a
repository that does not exist. The repo is now resolved once and both id and
name come from that one answer, with unresolved reported as `0`.

**A count is not an answer.** A lane with several PRs rendered only "N pull
requests", so a reader could not tell which one was red. `PullRequestFacts`
carries axes and a primary key because that is all a VERDICT needs; the stored
row's display identity (number, title, host slug) is now joined back on, and
each host axis renders through a lookup with an explicit fallback.

**An incomplete scan is not an empty one.** `evidence_scan_truncated` reached
the Rust DTO but never the frontend, so a lane whose rows all fell past the
per-issue scan bound was reported as "no evidence recorded" — an incomplete
read turned into a false assertion. Evidence is now one discriminated
`LaneEvidenceView`, and "none" is only claimed when the scan supports it.

Verified: cargo test --lib change_set (14 passed) and readiness (80 passed),
pnpm build, pnpm test (212 passed), npx tsc --noEmit, git diff --check.

Refs #175

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xz1Ts3AK1uNbeWZT6YFbzz

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 92775bb0a3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

{t("changeSet.laneCount", { count: state.changeSet.active_lane_count })}
</span>
</header>
{state.waves.map((wave, index) => (

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Render issue-scoped evidence

Whenever evidence is recorded with direction_id == 0—for example, host evidence for an unbound PR—the backend stores it only in issue_evidence and deliberately leaves virtual-lane summaries empty. This ready branch renders only state.waves, never state.changeSet.issue_evidence, so those records disappear from the overview; if there are no lanes, the panel instead reports the entire Change Set as empty. Render an issue-level evidence summary independently of the lane waves.

Useful? React with 👍 / 👎.

Comment on lines +308 to +311
<span>{axis(CI_KEYS, pr.ci.state)}</span>
<span>{axis(REVIEW_KEYS, pr.review.state)}</span>
<span>{axis(THREADS_KEYS, pr.threads.state)}</span>
<span>{axis(CONFLICT_KEYS, pr.conflict.state)}</span>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Render the lifecycle of each pull request

When a lane has multiple PRs and one is closed without merging, readiness produces the lane-level pr_closed_unmerged reason, but each row here renders only CI, review, thread, and conflict axes; the closed PR can therefore look completely green and the user still cannot identify which row caused the verdict. Fresh evidence after the prior identity finding is that the updated DTO now carries pr.lifecycle, but this renderer never consumes it; map that lifecycle exhaustively per row.

AGENTS.md reference: AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

Comment thread src-tauri/src/change_set.rs Outdated
Comment on lines +411 to +412
let rows = repo::list_evidence(db, thread_id, None, EVIDENCE_SCAN_LIMIT).await?;
let truncated = rows.len() as u64 >= EVIDENCE_SCAN_LIMIT;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Detect evidence truncation using an extra row

When an issue has exactly 200 evidence rows, this query returns all of them, yet >= EVIDENCE_SCAN_LIMIT marks the scan as truncated. The frontend consequently labels every zero-count lane as unscanned even though the scan was complete. Fetch one row beyond the display limit and set the flag only when that sentinel row exists, then summarize only the first 200.

Useful? React with 👍 / 👎.

Comment thread src-tauri/src/change_set.rs Outdated
Comment on lines +204 to +206
let directions = repo::list_directions(db, thread_id).await?;
let directions_by_id: HashMap<i32, &crate::store::entities::direction::Model> =
directions.iter().map(|row| (row.id, row)).collect();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Project scope from the same direction snapshot

When this collection overlaps proposal confirmation or materialization, readiness can build a virtual fact for a direction that did not exist at collection start, while this second direction read sees the newly created row. The projection then marks that lane materialized, attaches its new repo, branch, and dependencies, but retains the virtual fact's sentinel lifecycle, absent checkout sample, and verdict; deletions or scope changes during the long-running check collection produce the inverse mismatch. Carry the direction snapshot used by collect_facts_and_readiness into the projection, or version-check and retry rather than combining two generations.

Useful? React with 👍 / 👎.

Comment thread src/board/ThreadBoard.tsx Outdated
Comment on lines +197 to +200
const read =
deliveryView === "changeSet"
? api.issueChangeSet(request.threadId).then(deliveryFromChangeSet)
: api.issueReadiness(request.threadId).then(deliveryFromReadiness);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prevent delivery refreshes from overlapping

When the user switches between the Change Set and another issue tab before the current invoke finishes—or when a collection lasts into the next 60-second poll—the effect cleanup only suppresses the old response; it does not stop the backend command. This branch immediately starts the other collection, so issue_readiness and issue_change_set can probe the same worktrees concurrently, recreating the contention and unknown results that the new single-read design explicitly aims to prevent. Serialize or reuse the in-flight collection, or add real backend cancellation rather than merely discarding its response.

Useful? React with 👍 / 👎.

…hat was collected

Five more from Codex on #218. Two are structural and matter more than the count
suggests.

**The projection read a SECOND generation.** `collect_facts_and_readiness` read
the directions, then `project` read them again. A proposal confirmed or a lane
deleted between the two would produce a row half from each: a lane marked
`materialized` with a freshly created direction's repo, branch and
dependencies, still carrying the virtual fact's sentinel lifecycle, absent
checkout sample and verdict. This module's whole premise is that two
derivations can disagree — and it was doing exactly that with its own inputs.
The collection now returns the direction snapshot it consumed
(`CollectedIssue`), and the projection takes it rather than re-reading.

**Serializing the reads was not enough to stop the overlap.** Cancelling an
effect only discards the RESPONSE; the backend collection keeps running. A tab
switch mid-flight, or a poll tick landing on a long collection, still put two
collections on the same worktrees — reviving the probe contention the
single-read design exists to remove. Reads now chain: each waits for the
previous to settle before issuing. This does not claim to cancel the backend,
it prevents the second start.

**Truncation was off by one.** An issue with exactly 200 rows was scanned
completely, yet `>= LIMIT` called it truncated, which made every quiet lane
read "not fully scanned" when the counts were exact. The scan now reads one row
past the bound purely as a sentinel and summarizes only the first 200.

**A closed PR could look entirely green.** `pr_closed_unmerged` drives the lane
verdict on its own, but the row rendered only CI, review, threads and conflict
— never the lifecycle the DTO already carried. It is now shown first, and a
closed one is marked.

**Issue-level evidence had no home.** Rows recorded against the issue rather
than a lane are deliberately kept out of the lane summaries, so without
rendering them they simply vanished — and an issue with no lanes reported as
empty while still holding records. The empty state now carries the change set,
and both states render an issue-level line.

Verified: cargo test --lib change_set (14 passed) and readiness (80 passed),
pnpm build, pnpm test (213 passed), npx tsc --noEmit, git diff --check.

Refs #175

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xz1Ts3AK1uNbeWZT6YFbzz

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4cfb5fec9e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/board/ThreadBoard.tsx Outdated
Comment on lines +210 to +211
const read = deliveryReadChainRef.current.catch(() => {}).then(issue);
deliveryReadChainRef.current = read.catch(() => {});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Drop obsolete delivery reads before invoking them

When a collection exceeds the 60-second polling interval, or the user switches tabs/issues repeatedly while one is running, every invalidated effect still appends its captured issue callback to this chain; cleanup suppresses only its response. The newest visible request must wait for all obsolete collections, and if refreshes arrive faster than collections finish the queue grows without bound, leaving the panel loading or increasingly stale. Coalesce queued requests or check cancellation before invoking the next backend command.

Useful? React with 👍 / 👎.

Comment thread src/board/ChangeSetPanel.tsx Outdated
Comment on lines +251 to +252
const { fresh, stale, unknown } = changeSet.issue_evidence;
if (fresh + stale + unknown === 0) return null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle truncation in issue-level evidence

When an issue has more than 200 evidence rows and its direction_id == 0 rows fall beyond the scan window, issue_evidence is all zero while evidence_scan_truncated is true, so this return hides the issue-level evidence entirely. Unlike laneEvidenceView, this path never distinguishes a complete zero from an incomplete scan; derive an exhaustive issue-evidence view from both the counts and truncation flag and render an incomplete indication.

AGENTS.md reference: AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

Comment on lines +264 to +266
let depends_on = match direction {
Some(direction) => upstream_direction_ids(db, direction.id).await?,
None => Vec::new(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Project dependency edges from the collected snapshot

When proposal confirmation or re-proposal updates dependency edges during a long readiness collection, the lane verdict uses the edge state read by collect_lane, but this later query can read the replacement edges and display a different ordering beside that verdict. Fresh evidence after the earlier direction-snapshot fix is that dependency edges are still fetched separately here, and reused direction rows can have their edges changed without changing that snapshot; carry the collected edges in LaneFacts or version-check and retry.

Useful? React with 👍 / 👎.

return (
<article className="flex flex-col gap-2 rounded-[var(--radius-lg)] border border-border bg-surface px-3 py-2.5">
<div className="flex min-w-0 flex-wrap items-center gap-2">
<span className="min-w-0 truncate text-[12.5px] font-semibold text-ink">{lane.name}</span>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Translate synthetic lane names before rendering them

When the UI is using Chinese and readiness adds an issue-wide ask or unbound-PR contributor, the backend supplies the fixed English names issue ask and unbound PR, and this renders them verbatim among otherwise translated Change Set labels. These are application-owned states rather than user-provided lane names, so expose a virtual-lane discriminator or translation key and map it through the locale files before display.

AGENTS.md reference: AGENTS.md:L16-L16

Useful? React with 👍 / 👎.

…ply the truncation rule to issue evidence

Both are regressions from the previous commit.

**The serialization chain had no coalescing.** Fixing the overlap by chaining
reads meant every invalidated effect still appended its command to the queue —
cleanup suppressed the RESPONSE but not the call. Rapid tab or issue switching,
or a collection outliving the 60-second poll, would queue one obsolete backend
command per refresh, and the newest visible request would wait behind all of
them; the panel stays loading while the queue grows. Cancellation is now
re-checked at the FRONT of each queued turn, so an obsolete read becomes a
no-op rather than a collection nobody is waiting for.

**Issue-level evidence ignored the truncation flag** the lane rows had just
been taught to respect. All-zero counts from a truncated scan were rendered as
nothing at all — hiding the row asserts "there is none" just as strongly as
printing it would. It now takes the same three-way view.

Verified: pnpm build, pnpm test (214 passed), npx tsc --noEmit, git diff --check.

Refs #175

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xz1Ts3AK1uNbeWZT6YFbzz

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 691cfbfc98

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +430 to +432
let mut rows = repo::list_evidence(db, thread_id, None, EVIDENCE_SCAN_LIMIT + 1).await?;
let truncated = rows.len() as u64 > EVIDENCE_SCAN_LIMIT;
rows.truncate(EVIDENCE_SCAN_LIMIT as usize);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve refreshed evidence in bounded scans

When an issue has more than 200 evidence rows and an older row is re-confirmed, append_evidence updates that row's observed_at without changing its ID, while list_evidence still orders the bounded page by descending ID (src-tauri/src/store/repo.rs:8618-8640,8690-8693). This scan can therefore omit a newly refreshed fact; if that lane has any other row inside the window, the UI renders the resulting nonzero counts without the truncation warning and can show stale/unknown evidence despite a fresh observation. Select the bounded window by observation recency, or otherwise include refreshed rows before summarizing.

Useful? React with 👍 / 👎.

Comment thread src/board/ChangeSetPanel.tsx Outdated
Comment on lines +136 to +139
<span>{t("changeSet.wave", { index: index + 1 })}</span>
{index > 0 && <ArrowRight size={11} aria-hidden="true" />}
{index > 0 && (
<span className="normal-case tracking-normal font-normal">{t("changeSet.waveWaits")}</span>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Number executable waves independently of cycles

When a cycle precedes a downstream executable wave, index still counts the cycle even though the cycle header explicitly has no step number. For example, an A↔B cycle followed by C renders C as “step 2” and says it waits on the step above even though no step 1 exists; with an independent first wave it skips from step 1 to step 3. Compute the displayed ordinal from preceding parallel waves rather than the raw array position.

Useful? React with 👍 / 👎.

/// `reconciliation_for` reduced away.
fn lane_checkout(probed: &ProbedWorktree) -> LaneCheckout {
LaneCheckout {
repo_name: probed.repo.clone(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Localize the missing-repository checkout label

When a worktree's repository row cannot be resolved, probe_worktrees_for_direction supplies the English sentinel repo {id} (readiness.rs:1800-1803), and this new projection exposes it as repo_name for direct rendering in the Change Set. In the Chinese locale that degraded checkout row therefore contains an untranslated application-owned label; carry a missing-repository discriminator or ID and format the fallback through the locale files instead.

AGENTS.md reference: AGENTS.md:L16-L16

Useful? React with 👍 / 👎.

Comment on lines +260 to +263
let resolved_repo = match direction {
Some(direction) => resolve_repo(db, direction.repo_id).await,
None => None,
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve proposed lane scope before materialization

While a proposal is still pending or partially approved, its unmaterialized lanes already carry a repository, reason, base branch, and declared dependencies, but this projection treats every lane without a direction row as having no scope. Those lanes consequently render without the repo/rationale and are placed as independent waves even when the proposal says they wait on another lane, defeating the overview precisely before confirmation. Carry the collected proposal snapshot into LaneFacts or the projection instead of deriving all scope exclusively from materialized directions.

Useful? React with 👍 / 👎.

A cycle wave renders without a step number, because it is not a step anyone can
take — but it still consumed an array index, so the executable wave after an
A<->B cycle read "step 2, waits on the step above" when no step 1 had been
shown, and an independent first wave made the numbering jump 1 -> 3. The
ordinal is now counted over the parallel waves only.

Verified: pnpm build, pnpm test (215 passed), npx tsc --noEmit, git diff --check.

Refs #175

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xz1Ts3AK1uNbeWZT6YFbzz

Copy link
Copy Markdown
Owner Author

CI note — rust-test (macos-latest) is red on 691cfbf, and it is not this PR.

The failure is readiness::tests::bounded_check_evidence_keeps_observed_failures_sticky, left: Failing, right: Passed. That is the pre-existing macOS/under-load spawn flake: when the runner is tight, a check loses its spawn, run_checks_with_timeout_report records it as a genuine failure, and the suite reads Failing where it expected Passed.

Evidence it is not from this branch:

  • This PR's readiness.rs diff touches nothing in the check-spawn path — no BoundedCheckOutcome, run_checks_with_timeout_report, saw_failure, or CheckEvidence lines. The change here is a return-type refactor (CollectedIssue) plus checkout/repo projection.
  • The test passes 5/5 in isolation locally, and reproduces only under concurrent load — the flake's signature.

#216 is the fix for exactly this, by routing spawn failures to NotProduced instead of Completed { status: "fail" }. This branch is based on main, which does not have it yet. Landing #216 should stop this from reddening unrelated PRs.

I have not re-run the job — the flake is real and worth landing #216 for rather than papering over with a retry.


Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e0d72e5de7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +128 to +130
for (const remaining of blockers.values()) {
for (const position of stuck) {
remaining.delete(position);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep dependents of a cycle blocked

When A and B form a cycle and C depends on A, deleting the cycle positions from every remaining blocker set makes the next iteration emit C as a normal parallel wave; the panel consequently renders C as executable “Step 1” with no waiting warning, even though its upstream cycle remains unresolved. Fresh evidence after the prior cycle finding is that the revised grouping now separates C from the cycle but explicitly promotes it after removing the cyclic blocker; preserve a distinct blocked-after-cycle state instead.

AGENTS.md reference: AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

Comment on lines +225 to +227
let pull_request_rows = repo::list_pull_requests_for_thread(db, thread_id)
.await
.unwrap_or_default();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Propagate pull-request lookup failures

If list_pull_requests_for_thread fails because the database is locked, corrupt, or otherwise unavailable, unwrap_or_default() silently turns the failure into an empty result. The command then succeeds while every tracked PR is rendered as “record missing,” which is materially different from being unable to read the records; propagate the error so the panel reports a load failure instead.

AGENTS.md reference: AGENTS.md:L17-L17

Useful? React with 👍 / 👎.

if repo_id == 0 {
return None;
}
let row = repo::get_repo(db, repo_id).await.ok().flatten()?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Distinguish repository lookup errors from missing rows

When get_repo returns an actual database error, .ok() converts it into the same None used for a dangling repo_id. The resulting successful response falsely reports the repository as unresolved and disables the lane opener, masking an operational failure as missing data; return Result<Option<_>> and propagate Err while still tolerating a successful lookup that returns no row.

AGENTS.md reference: AGENTS.md:L17-L17

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Roadmap][R1-05] 实现 Issue Change Set 与聚合 readiness

2 participants