Skip to content

Add deep-review skill for iterative code review#5

Merged
intendednull merged 2 commits into
mainfrom
claude/deep-review-skill-HXIs1
Mar 26, 2026
Merged

Add deep-review skill for iterative code review#5
intendednull merged 2 commits into
mainfrom
claude/deep-review-skill-HXIs1

Conversation

@intendednull
Copy link
Copy Markdown
Owner

Introduces a new /deep-review slash command that performs multi-pass
code review: flags potential issues, spawns parallel investigation
agents, presents a triage table, fixes user-selected issues, and
loops until clean.

https://claude.ai/code/session_01Mihs2Z1JUCJNc3DAm5MXQw

claude added 2 commits March 26, 2026 14:21
Introduces a new /deep-review slash command that performs multi-pass
code review: flags potential issues, spawns parallel investigation
agents, presents a triage table, fixes user-selected issues, and
loops until clean.

https://claude.ai/code/session_01Mihs2Z1JUCJNc3DAm5MXQw
Skills must be at .claude/skills/<name>/SKILL.md (not flat .md files)
and use `user-invocable` (hyphen) not `user_invocable` (underscore).

https://claude.ai/code/session_01Mihs2Z1JUCJNc3DAm5MXQw
@intendednull intendednull merged commit 8bd9c35 into main Mar 26, 2026
@intendednull intendednull deleted the claude/deep-review-skill-HXIs1 branch March 26, 2026 14:27
intendednull added a commit that referenced this pull request Mar 28, 2026
…cast, test gaps

- Storage role: log event storage and DB query failures instead of
  silently dropping (#3, #4)
- Network actor: publish responses back via gossipsub instead of only
  logging, add 5s timeout, log malformed messages (#5, #7)
- Client authorize_workers: broadcast GrantPermission events to network
  so workers actually learn they're authorized (#9)
- Heartbeat actor: log state actor unavailability and departure send
  failures (#11, #17)
- Storage role tests: add dedup, unknown server, unknown channel edge
  cases (#13)
- Deep review skill: expand test gaps category to cover error-path and
  edge-case coverage

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
intendednull pushed a commit that referenced this pull request Apr 5, 2026
Fixes from three-agent deep review (PR comments, research papers,
plan analysis):

**Critical: Equivocation detection**
- Added ChainStatus::Forked variant for same-seq-different-hash
- Added Equivocation Detection section in Section 4 with freeze/
  propagate/alert response protocol
- Referenced 2P-BFT-Log paper (arXiv:2307.08381)

**Governance fixes**
- Default vote threshold changed from Unanimous to Majority (PR #4)
- Bootstrap sequence updated for majority default
- Auto-apply mechanics clarified: no separate event, state change
  inferred during materialization (PR #5)
- Vote structurally references proposal via EventHash field (PR #6)
- KickMember/RevokeAdmin now clean up pending votes and re-evaluate
  all proposals (cascade thresholds) via cleanup_votes_and_reevaluate
  and reevaluate_all_proposals helpers
- SetVoteThreshold re-evaluates all pending proposals
- 5 new governance tests for vote cleanup and threshold cascading

**Permission fixes**
- GrantPermission/RevokePermission/RenameServer/SetServerDescription
  require is_admin check directly (not via ManageRoles permission)
- SendMessages permission now enforced for Message/EditMessage/
  DeleteMessage/Reaction events (was defined but never checked)
- Permission enum has doc comments explaining each variant

**Type fixes**
- Channel.pinned_messages: HashSet<String> → HashSet<EventHash>
- Event struct has explicit #[derive(Clone, Debug, Serialize, Deserialize)]
- Archival tolerance design goal reworded for append-only model

**Incremental apply safety**
- Added soft-dep arrival re-materialization note: when a previously
  missing dep arrives and creates new causal edges, re-materialize
  from the DAG to maintain consistency

**Appendix A: Known Issues and Future Work**
- Hash-grinding on governance events
- Optimized incremental apply after soft-dep arrival
- SyncProvider permission evaluation
- Versioning strategy
- Dynamic channel model

https://claude.ai/code/session_015u9tV4Abp1kKXvCVZ5iMfv
intendednull added a commit that referenced this pull request Apr 26, 2026
lifecycle, fix IrohBlobStore spec drift, track 4 new follow-ups

Round 2 review (two fresh agents) verified all 15 round-1 fixes
land cleanly with no regressions, then surfaced 8 new findings
(0 critical, 3 medium, 5 low) by widening scope to cross-component
interactions, perf, and API surface.

Fixed inline (trivial doc / spec):

- Add an "Actor coordination signal" row to the spec decision tree
  + CLAUDE.md table covering tokio::sync::watch / oneshot /
  broadcast / Notify, with the explicit rule that
  tokio::sync::Mutex is forbidden for business state on the same
  terms as std/parking_lot Mutex. Closes the spec gap that left
  contributors without guidance on async channels. (round-2 #3)
- Reconcile spec § 184 with the corrected IrohBlobStore comment
  (round-1 fixed the code, missed the spec). The blob store is
  not an iroh-callback boundary — it's an interim stub. The relay-
  status timestamp Mutex stays in the iroh boundary list. (round-2 #4)
- Document the web `_event_loop` drop pattern in `crates/web/src/app.rs`
  so future readers see explicitly that the actor System is process-
  scoped on web (page reload tears everything down) and that any
  actor needing pre-close cleanup must route via `beforeunload`,
  not Drop. (round-2 #8)

Tracked as new follow-ups in spec § Follow-up work:

- F5. SearchActor head-of-line + rebuild-storm fix. Rebuild blocks
  Query in FIFO order; the rebuild Effect has no debounce. Fix is
  chunked-Rebuild + Debounce<Rebuild> wrap. (round-2 #1, #2 — Med)
- F6. Browser-tier coverage for SearchIndexHandle consumers. The
  spawn_local + Effect path has no wasm-pack test. (round-2 #6)
- F7. Sealed ClientSpawner to narrow the system() API surface,
  rather than exposing the full SystemHandle. (round-2 #7)
- F8. Search-query debouncing-flicker fix via generation tag or
  Leptos Resource migration. (round-2 #5)

Each follow-up has a "Trigger:" line naming the dedicated PR title.

`just check` green: clippy zero warnings, 1003+ tests pass, WASM
compile clean. Loop terminates here per the user's two-round cap;
no Critical issues remain.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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