Skip to content

feat: name the exact checks and reviews behind ci-changed/review-changed details - #12

Merged
diegomarino merged 5 commits into
mainfrom
claude/opaque-check-digest-todos-06xc35
Jul 9, 2026
Merged

diegomarino merged 5 commits into
mainfrom
claude/opaque-check-digest-todos-06xc35

Conversation

@diegomarino

Copy link
Copy Markdown
Owner

Resolves the two TODO(v0.2) notes in lib/cli.mjs detail rendering: ci-changed and review-changed details previously exposed only an opaque digest transition, forcing agents to re-query GitHub to learn what changed.

What changed

Persisted normalized summaries. The PR fingerprint now stores two compact, detail-only fields in the snapshot:

  • ciChecks — sorted {name, status, conclusion} rows (normalizes both CheckRun and StatusContext rollup shapes)
  • reviewSummary — sorted {author, state, submittedAt, commit} rows from latestReviews

Named detail breakdown. With --detail, ci-changed / review-changed (and the updated catch-all) keep their digest from/to and gain added / removed / changed arrays:

{
  "class": "ci-changed",
  "field": "ci",
  "from": "b6e7d0bc0ba1",
  "to": "632f7f45b0b2",
  "added": [{ "name": "lint", "status": "IN_PROGRESS", "conclusion": "" }],
  "removed": [{ "name": "docs", "status": "COMPLETED", "conclusion": "SUCCESS" }],
  "changed": [{ "name": "build", "from": { "status": "COMPLETED", "conclusion": "FAILURE" }, "to": { "status": "COMPLETED", "conclusion": "SUCCESS" } }]
}

opaque: true remains only as the fallback when the detail cannot name the change (a from side written before summaries existed, or a digest diff the summary does not explain).

Compatibility

  • No phantom deltas on upgrade: comparableFingerprint drops the summaries, so a pre-summary snapshot compares equal to a current fetch of the same state; the first tick just persists the summaries.
  • Stable delta ids: the summaries are excluded from deltaIdentity, so the same observed change yields the same content-addressed id from legacy and new snapshots (cross-monitor dedupe survives mixed versions). Verified end-to-end.
  • Frozen digests: canonicalizeCiRollup now derives from summarizeCiRollup (digest and summary can never disagree) and a test freezes the digest values byte-identical to the pre-summary implementation.
  • Additive contract: DELTA_DETAIL_FIELDS gains changed; no schemaVersion bump per the documented policy.

Docs

docs/contract.md (detail shapes, fingerprint table, API exports), --help-json description, and the agent-facing guides (RUNBOOK.md, docs/watch-loop-prompt.md, docs/usage.md) now describe the named breakdown and the opaque fallback. The README example fixtures are documented as exercising the pre-summary fallback path, so no SVG regeneration was needed.

Testing

  • npm run check green: lint, prettier, 161 tests (6 new: summarizer normalization/sorting, frozen digests, comparable stripping, named CI/review details, legacy-snapshot upgrade without phantom deltas, id stability).
  • Manual end-to-end with the real binary and a stubbed gh over two ticks: named diff output, legacy-snapshot upgrade (exit 0, zero deltas, summaries persisted), opaque fallback, and identical delta ids across snapshot versions.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PGgDzjgxoGajZU7sHJTp81


Generated by Claude Code

claude added 3 commits July 8, 2026 21:43
…ged details

Persist normalized summaries in the PR fingerprint — ciChecks (sorted
{name, status, conclusion} rows) and reviewSummary (sorted {author,
state, submittedAt, commit} rows) — so --detail can report what actually
changed instead of only an opaque digest transition, letting an agent
act without re-querying GitHub.

- ci-changed / review-changed (and the updated catch-all) details gain
  added/removed/changed arrays when both fingerprint sides carry the
  summaries; opaque: true remains only as the fallback for snapshots
  written before summaries existed or diffs the summary cannot explain.
- canonicalizeCiRollup now derives from summarizeCiRollup, so the digest
  and summary can never disagree; digests are frozen by test to stay
  byte-identical with pre-summary snapshots.
- comparableFingerprint drops the summaries, keeping change comparison
  and content-addressed delta ids stable across the upgrade (no phantom
  deltas, same ids from legacy and new snapshots).
- Contract: DELTA_DETAIL_FIELDS gains 'changed'; docs/contract.md and
  --help-json describe the named breakdown and the opaque fallback.

Resolves the two TODO(v0.2) notes in lib/cli.mjs detail rendering.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PGgDzjgxoGajZU7sHJTp81
RUNBOOK class table, the watch-loop prompt doc, and usage.md now mention
that --format json --detail names the exact checks/reviews that changed,
so acting agents know the second GitHub query is no longer needed.

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

@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: 10b172304b

ℹ️ 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 lib/cli.mjs
claude added 2 commits July 8, 2026 21:55
Two CI rollup rows can share a name (a CheckRun and a StatusContext, or
duplicate check runs); keying the diff by name silently dropped rows and
misreported the breakdown. Refuse to name the diff when either side
repeats a key and mark the digest transition opaque: true instead, so a
non-opaque detail stays authoritative.

Addresses the Codex review finding on PR #12.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PGgDzjgxoGajZU7sHJTp81
Conflict in docs/contract.md programmatic API table: kept main's rows
(gh-delta/list, gh-delta/registry, list/registry contract symbols) and
re-added summarizeCiRollup/summarizeReviews to the fingerprint row.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PGgDzjgxoGajZU7sHJTp81
@diegomarino
diegomarino merged commit d948d3e into main Jul 9, 2026
3 checks passed
@diegomarino
diegomarino deleted the claude/opaque-check-digest-todos-06xc35 branch July 9, 2026 14:36
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.

2 participants