Split retrospective analysis into main first-pass and Fable review#284
Open
ikuwow wants to merge 8 commits into
Open
Split retrospective analysis into main first-pass and Fable review#284ikuwow wants to merge 8 commits into
ikuwow wants to merge 8 commits into
Conversation
Replace the asymmetric global-auto-issue / project-AskUserQuestion routing with per-finding AskUserQuestion for both scopes, and replace the 4-step countermeasure priority list with a 5-destination taxonomy (existing-rule-edit, existing-skill-update, mechanize, new-rule, new-skill) tagged explicitly on each finding. Preserve the sanitize rule for global issues (now stated up front in Step 2) and add a prompt-fatigue split for sessions with many findings. Co-authored-by: Claude <Opus 4.7 (1M context)> <[email protected]>
Resolve gaps flagged by an outside review of the routing rewrite: the apply-now branch context was unspecified (session end could be on the default branch or an unrelated feature branch), and the create-issue granularity was ambiguous between per-finding and per-session issues. Add a branch gate that refuses apply-now on the default branch and leaves feature-branch writes uncommitted, batch approved findings by scope into one session-level issue each, and note that the dotfiles cwd edge case still uses Other rather than lifting the global apply-now ban. Co-authored-by: Claude <Opus 4.7 (1M context)> <[email protected]>
Code review flagged three issues. The branch-gate detection compared `git symbolic-ref refs/remotes/origin/HEAD` (which returns `refs/remotes/origin/main`) to `git rev-parse --abbrev-ref HEAD` (which returns `main`), so the two strings never matched and the gate silently failed open. Normalize both to short branch names and show the shell one-liner explicitly. Also split the "Global finding" multi-clause bullet into parent + rationale/exception children per the bullet-per-sentence rule, and restructure the `skip` / `Other` outcome lines as prose so they no longer sit as trailing bullets under the `create issue` list. Co-authored-by: Claude <Opus 4.7 (1M context)> <[email protected]>
Main session now produces first-pass findings plus a factual digest from in-memory context (no transcript read), and a Fable subagent reviews them against a mandatory small sample of transcript slices, returning per-finding confirm/adjust/reject verdicts. Additions Fable proposes go through a delta-only round 2 (2-round hard cap), and any dissent from an overridden Fable verdict is preserved for Step 4's routing so the user can arbitrate. This cuts the transcript read volume from the whole jsonl to targeted slices while keeping an outside-perspective check on main's self-analysis. Co-authored-by: Claude <Opus 4.7 (1M context)> <[email protected]>
Implementer preserved Step 4 content byte-for-byte on instruction, which left the intro line referring to "the subagent's output" and tags "from Step 1" — both stale under the new pipeline where Step 3 finalizes findings that may carry tag adjustments from Step 2/3. Retarget the intro at Step 3's output and note the dissent-surfacing path so the routing step matches the flow above it. Co-authored-by: Claude <Opus 4.7 (1M context)> <[email protected]>
…nc intro Code review flagged three consistency issues. Fable's Step 2 prompt mandated slice-sampling and forbade whole-file reads, but analysis.md's Reading-the-transcript still offered a whole-file shortcut for small files — align by dropping the shortcut. Findings about the currently- in-progress turn (not yet in the transcript) were falling through to reject on missing evidence — add a fourth unverifiable verdict Fable returns in that case, and route it in Step 3 as kept-with-caveat, not dissent. Rewrite analysis.md's opening line so it addresses both consumers named in Reader context instead of assuming a single transcript reader. Co-authored-by: Claude <Opus 4.7 (1M context)> <[email protected]>
# Conflicts: # claude/skills/retrospective/SKILL.md
1 task
Only recorded dissent was being shown to the user, so when main accepted Fable's verdict the outside review disappeared from the routing choice — the user picked a route without seeing what Fable concluded. Require every AskUserQuestion to include a compact Fable summary line (verdict plus one-line reason) regardless of acceptance, and keep the extra dissent display for disagreements. Co-authored-by: Claude <Opus 4.7 (1M context)> <[email protected]>
Owner
Author
Agent Teams direction — investigation notes (for tomorrow)Merge held. Retrospective didn't converge into real dialogue with the SendMessage-based subagent design (Fable review is one-shot plus optional defense round; main is the only decision-maker; no severity negotiation or counter-argument exchange). Agent Teams looked worth investigating as a replacement. What's already available in Claude Code (verified against
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 1 (#283) kept the original executor: a Fable subagent reads the whole session jsonl from disk and produces findings one-shot. That duplicates work — the main session already has the same context in memory — and re-pays the read cost on every session end even though the jsonl is typically several MB. This PR flips the executor so main does the first pass in-memory, and Fable reviews the resulting findings against a small mandatory sample of transcript slices instead of the whole file.
Why the shape it took
Splitting analyst from reviewer moves total transcript I/O from "whole file every time" to "a few slices plus targeted spot-checks", while keeping an outside perspective that catches motivated severity-downgrading. Main owns the initial slate and the final decision; Fable is the challenger with a hard 2-round cap and delta-only round 2 (Fable's "add missing" proposals get one defense pass, not a full re-review).
Self-bias mitigation: any finding where main overrides Fable's
rejectoradjustcarries Fable's dissent into Step 4'sAskUserQuestion, so the user arbitrates disputed cases explicitly. The rubber-stamp path stays only onconfirm.Compaction handling: an earlier reviewer suggestion to detect compaction via an
isCompactSummaryjsonl field was dropped aftergrepacross all local~/.claude/projects/**/*.jsonlreturned zero matches for that field name. Instead Fable's slice sample is mandatory every session, so the coverage check runs whether the digest is complete or lossy.Verification
Static:
pre-commit run --files claude/skills/retrospective/SKILL.md claude/skills/retrospective/analysis.md→ all applicable hooksPassed; JSON/YAML/line-count budget hooksSkippedas N/Agit grep -n 'Step 1\|Step 2\|Step 3\|Step 4' claude/skills/retrospective/SKILL.md→ step cross-references all consistent, no dangling references to the deleted old Step 1 delegation templateSendMessagetool schema verified against the current session'sToolSearch select:SendMessageresult — the round-2 continuation mechanism exists at time of writingEnd-to-end:
/retrospectiveonce from a throwaway session with all findings routed asskip, and confirm (a) the 2-round cap fires when Fable emitsadd missing, (b) Fable's mandatory slice sample actually happens, (c) dissent surfaces toAskUserQuestionwhen main overrides a Fable verdict. Deferred out of this PR becauseclaude/skills/retrospective/*is symlinked into~/.claude/, so any real invocation mutates the live skill for every concurrent Claude Code session on this host, and non-skippaths would open real GitHub issuesBase branch
Stacked on
retrospective-routing(PR #283) — merge #283 first, then this. GitHub will re-target the base tomainautomatically after #283 lands.