Skip to content

feat(maintainer-workflows): cross-workflow review memory + backfill#1458

Merged
Wirasm merged 1 commit intodevfrom
feat/maintainer-review-memory
Apr 28, 2026
Merged

feat(maintainer-workflows): cross-workflow review memory + backfill#1458
Wirasm merged 1 commit intodevfrom
feat/maintainer-review-memory

Conversation

@Wirasm
Copy link
Copy Markdown
Collaborator

@Wirasm Wirasm commented Apr 28, 2026

Summary

  • Problem: maintainer-standup briefs had no awareness of which PRs had already been triaged via maintainer-review-pr. Reviewed PRs kept re-appearing in P1-P4 with no marker, leading the maintainer to re-skim the same items each morning.
  • Why it matters: when a PR is reviewed and the contributor pushes new commits in response, the maintainer needs to know: (a) I already weighed in, (b) but the diff has moved since — re-review may be warranted. Today the brief gives neither signal.
  • What changed: new gitignored state file .archon/maintainer-standup/reviewed-prs.json shared between the two workflows. Writer node in maintainer-review-pr.yaml records each run; standup's read-context loads it; standup synthesizer prompt surfaces "✓ reviewed Nd ago" markers in P1-P4 with a staleness flag when the contributor pushed since.
  • What did NOT change (scope boundary): no engine code, no new YAML schema, no schema migrations. Pure .archon/ user content + a small inline persist node + a one-shot backfill script. Other workflows untouched.

Three pieces

1. Writer — maintainer-review-pr.yaml record-review node

Inline bun script that runs after whichever branch fires (post-review / post-decline / approve-unclear) with trigger_rule: one_success. Reads $gate.output.verdict, the PR number from $ARTIFACTS_DIR/.pr-number, and the workflow run ID; upserts an entry into reviewed-prs.json. The report node's dependency moved from [post-review, post-decline, approve-unclear] to [record-review] so the state-write happens before the run terminates.

2. Reader — maintainer-standup-read-context.ts

Loads reviewed-prs.json into a new reviewed_prs field on the standup gather output. Same pattern as prior_state and recent_briefs. Empty object on first run / missing file.

3. Surface — maintainer-standup.md Phase 2h

New rule appended to Phase 2 instructing the synthesizer to mark PRs:

  • Reviewed (review branch) → ✓ reviewed Nd ago
  • Declined (decline / needs_split) → ✓ declined Nd ago
  • Triaged inconclusively → ✓ triaged Nd ago (unclear)

And a staleness check: compare reviewed_at to the PR's updatedAt; if the contributor pushed since, append ⚠ contributor pushed since so the maintainer knows the prior review may need re-running.

Plus: one-shot backfill script

.archon/scripts/maintainer-standup-backfill-reviews.ts scans the maintainer's GitHub comments in the last 7 days, pattern-matches ## Review Summary / direction-clause-citation / split-up wording, and populates reviewed-prs.json from history. Idempotent — workflow-recorded entries take precedence over backfilled ones (the writer node is authoritative; pattern-matching is a best-effort historical reconstruction). Uses 64MB maxBuffer on the gh exec because --paginate over 7 days easily exceeds Node's default 1MB.

Validation Evidence (required)

archon validate workflows maintainer-review-pr   # → ok
archon validate workflows maintainer-standup     # → ok
bun run format:check                             # → clean
bun run lint                                     # → clean

# Backfill verified end-to-end:
$ bun run .archon/scripts/maintainer-standup-backfill-reviews.ts
Scanning wirasm's comments on coleam00/Archon since 2026-04-21T...
Scanned 363 comments. 18 authored by wirasm matched a review/decline pattern.
Unique PRs: 17.
Backfilled 17 new entries (skipped 0 that already had workflow-recorded entries).
Total tracked: 17.

$ jq '. | to_entries | map({pr: .key, verdict: .value.gate_verdict})' \
  .archon/maintainer-standup/reviewed-prs.json
[
  {"pr":"1426","verdict":"review"},
  {"pr":"1420","verdict":"review"},
  {"pr":"1414","verdict":"review"},
  ...
]

17 PRs populated — exactly matches the count from yesterday's review session (14 reviews + 1 decline + 2 retries = 17 unique PRs). Spot-checked the entries against the actual workflow runs.

Security Impact (required)

  • New permissions/capabilities? No.
  • New external network calls? Backfill script only — gh api repos/.../issues/comments. Same scope as existing standup gather; uses the same gh auth.
  • Secrets/tokens handling? Unchanged.
  • File system access? Writes one new file under .archon/maintainer-standup/ (gitignored, run-scoped path).

Compatibility / Migration

  • Backward compatible? Yes. reviewed-prs.json is purely additive; absent file → empty object → no markers in the brief (current behavior). Existing maintainer-review-pr runs continue to work; the new record-review node only adds a state-write. The report node's dependency change ([post-review, post-decline, approve-unclear][record-review]) is internal — the report content is unchanged.
  • Config/env changes? No.
  • Database migration? No.
  • One-time backfill — opt-in. Run bun .archon/scripts/maintainer-standup-backfill-reviews.ts to seed the file from the last 7 days of comments. Without it, the file populates from new runs going forward.

Human Verification (required)

  • Verified scenarios:
    • Backfill on real comments → 17 PRs correctly populated, verdicts inferred correctly (15 review, 1 decline, 1 needs_split based on the actual content of yesterday's posted comments).
    • First-run / missing file behavior — read-context.ts returns reviewed_prs: {}; standup brief shows no markers (correct: no prior runs to surface).
    • Workflow YAML validation — both maintainer-review-pr and maintainer-standup validate cleanly from the subfolder.
    • Idempotency — running backfill twice doesn't duplicate entries (existing keys are preserved when source: 'workflow', and pattern-matched entries upsert latest-wins).
  • Edge cases checked:
    • Repo without an origin remote — backfill script exits with a clear error before any work.
    • 7-day comment volume on an active repo — confirmed 363 comments parse cleanly with 64MB maxBuffer; default 1MB would have ENOBUFS'd, which is why the explicit cap was added.
    • Body-text patterns that overlap (Conflicts with direction.md could appear in non-decline contexts) — pattern-match is best-effort by design; workflow-recorded entries supersede backfilled ones via existing[num] check on merge.
  • What was not verified:
    • The synthesizer actually rendering the markers in tomorrow's brief (need a real morning run with reviewed_prs populated). Today's standup already ran before this PR was authored, so the next run will be the first to read the new field.
    • Staleness signal end-to-end — depends on the synthesizer comparing two timestamps; the prompt instructions are explicit but the LLM may render imperfectly on first runs.

Side Effects / Blast Radius (required)

  • Affected subsystems: maintainer-review-pr (new node + dependency reorder), maintainer-standup (new field in gather + new rendering rule). No engine code touched.
  • Potential unintended effects:
    • The report node now depends on record-review instead of the three branch-end nodes directly. If record-review fails (file write error, JSON parse on corrupt existing file), report would skip. Mitigation: the writer try/catches the JSON parse and falls back to reviewed = {}; the only realistic failure is a permission error on the .archon dir, which would be diagnosable from the bash node's stderr.
    • Backfill script is run-once but technically re-runnable. Idempotent merge logic means re-runs don't duplicate, only update timestamps for backfilled entries.
  • Guardrails: workflow validation runs in CI; the new state file is gitignored, so it can't accidentally land in commits.

Rollback Plan (required)

  • Fast rollback: git revert <merge-sha> — single commit, 5 files. The new state file remains on disk locally but goes unused (no reader, no writer); cleanup is rm .archon/maintainer-standup/reviewed-prs.json if the user wants tidy disk.
  • Feature flags: none.
  • Observable failure symptoms: workflow validation failure (CI catches), or record-review failing → report skipped → workflow status failed. Either is loud and resumable.

Risks and Mitigations

  • Risk: record-review runs after the public-facing gh pr comment. If recording fails after the comment posted, the comment is on GitHub but the standup forgets it ever happened — same morning re-skim problem the feature was meant to solve.
    • Mitigation: writer uses defensive parsing (try/catch on existing file read); only realistic failure is a write-permission error on .archon/maintainer-standup/, which is rare. Future improvement: also stash a copy in $ARTIFACTS_DIR/ so it can be replayed if the canonical file is unwritable.
  • Risk: backfill pattern-matching produces false positives — a comment containing "## Review Summary" that wasn't actually a maintainer review (e.g., quoted in a thread).
    • Mitigation: filter is pinned to author == ghHandle. False positives only possible when the maintainer themselves authored a non-review comment containing the marker text — unlikely but possible. Mitigation if it surfaces: tighten the regex to also require the maintainer-review-pr footer line Reviewed via maintainer-review-pr workflow.
  • Risk: staleness signal is subjective. "Pushed since" includes label changes / draft toggles, not just commits — could produce false alarms.
    • Mitigation: synthesizer prompt instructs to flag staleness only when the gap is "real and meaningful," letting the LLM use judgment. If false alarms become a pattern, easy to tighten by checking only the headRefOid or commit-only push events via a more specific gh query.

Linked Issue

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced PR review tracking to persist review history with timestamps and verdicts
    • Added automatic detection to flag when contributors push changes after a PR review
    • Introduced backfill capability to recover historical review data from recent comments
  • Chores

    • Updated configuration to exclude internal review metadata from version control

The maintainer-standup brief had no signal for "I already triaged that
PR via maintainer-review-pr 2 days ago" — it just kept listing reviewed
PRs in P1-P4 with no acknowledgement of prior work. Result: maintainer
ends up re-skimming the same PR several mornings in a row.

This adds a shared persistent state file at:

  .archon/maintainer-standup/reviewed-prs.json (gitignored, per-maintainer)

shape:

  {
    "1338": {
      "reviewed_at": "2026-04-27T16:34:57Z",
      "gate_verdict": "review",     // review | decline | needs_split | unclear
      "run_id": "..."
    },
    ...
  }

Three pieces:

1. WRITER — new `record-review` script node in maintainer-review-pr.yaml,
   runs after whichever branch fired (post-review / post-decline /
   approve-unclear) with trigger_rule: one_success. Inline bun script;
   reads $gate.output.verdict, $ARTIFACTS_DIR/.pr-number, and
   $WORKFLOW_ID; appends/upserts the entry. report node now depends on
   record-review so the state write happens before the run completes.

2. READER — read-context.ts loads reviewed-prs.json into a new
   reviewed_prs field on the standup gather output. Same pattern as
   prior_state and recent_briefs.

3. SURFACE — maintainer-standup command file gets a Phase 2h rule:
   when listing PRs in P1-P4 / Polite-decline sections, append:
     - "✓ reviewed Nd ago" for review-branch entries
     - "✓ declined Nd ago" for decline / needs_split branches
     - "✓ triaged Nd ago (unclear)" for unclear branch
   and a STALENESS marker — compare reviewed_at to PR's updatedAt; if
   contributor pushed since the prior review, append
   "⚠ contributor pushed since" so the maintainer knows the prior pass
   may need to be re-run.

Plus a one-shot backfill script:

  .archon/scripts/maintainer-standup-backfill-reviews.ts

Scans the maintainer's gh comments in the last 7 days, pattern-matches
"## Review Summary" / direction-clause-citation / split-up wording, and
populates reviewed-prs.json. Idempotent; existing entries (from real
workflow runs) take precedence over backfilled ones (the writer-node
record is more authoritative than a body-pattern guess). Uses 64MB
maxBuffer on the gh exec because --paginate over 7 days of an active
repo's comments easily exceeds Node's default 1MB.

Backfill verified: 363 comments scanned, 18 matched, 17 unique PRs
populated — exactly the 17 PRs we reviewed via the workflow yesterday.

The new state file is gitignored alongside the existing per-maintainer
files (profile.md, state.json, briefs/).
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR introduces a cross-workflow review metadata persistence system for the maintainer standup. It adds a backfill script to scan GitHub comments and infer review verdicts, updates the maintainer-review workflow to record reviews in shared state, and extends the read-context script to load persisted review history for downstream synthesis.

Changes

Cohort / File(s) Summary
Review Recording & Persistence
.archon/workflows/maintainer/maintainer-review-pr.yaml, .archon/scripts/maintainer-standup-read-context.ts, .archon/commands/maintainer-standup.md
Workflow now records PR review metadata (verdict, timestamp, run ID) to .archon/maintainer-standup/reviewed-prs.json via new record-review node. Read-context script loads this persisted data and surfaces reviewed_prs in output. Spec documents new analysis phase for appending status markers and staleness checks based on review history.
Historical Review Backfill
.archon/scripts/maintainer-standup-backfill-reviews.ts
New executable script backfills review metadata by scanning maintainer's GitHub comments from past 7 days, infers verdicts via body-pattern matching (e.g., "Review Summary" → review, conflict phrases → decline), and writes results to reviewed-prs.json, respecting existing workflow-recorded entries.
Configuration
.gitignore
Adds ignore rule for .archon/maintainer-standup/reviewed-prs.json to prevent git tracking of generated review metadata.

Sequence Diagram

sequenceDiagram
    participant WF as Maintainer-Review<br/>Workflow
    participant FS as File System<br/>(reviewed-prs.json)
    participant RC as Read-Context<br/>Script
    participant BP as Backfill<br/>Script
    participant GH as GitHub API

    WF->>FS: record-review node<br/>persists verdict, timestamp, run_id
    Note over FS: reviewed-prs.json updated<br/>with PR entry

    BP->>GH: scan maintainer's comments<br/>past 7 days
    GH-->>BP: comment bodies & metadata
    BP->>BP: infer verdict from patterns<br/>(review/decline/needs_split)
    BP->>FS: load existing reviewed-prs.json
    BP->>FS: merge backfill results<br/>(skip existing workflow entries)

    RC->>FS: read reviewed-prs.json
    FS-->>RC: parsed review history
    RC->>RC: include reviewed_prs<br/>in output
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A hop through time, reviews now stay,
Backfill the past, persist today,
Cross-workflow memory, strong and bright,
Verdicts recorded, workflow-wright!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately summarizes the main change: introducing cross-workflow review memory and a backfill mechanism for the maintainer-standup workflow.
Description check ✅ Passed The description comprehensively covers all required sections from the template: clear problem/solution, architecture changes, validation evidence, security impact, compatibility, human verification, side effects, rollback plan, and risks with mitigations.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/maintainer-review-memory

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Wirasm Wirasm merged commit 2220ffe into dev Apr 28, 2026
3 of 4 checks passed
@Wirasm Wirasm mentioned this pull request Apr 29, 2026
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.

1 participant