Skip to content

feat: implement Deferral signal detector#2

Merged
Joi merged 3 commits into
Joi:mainfrom
mvanhorn:feat/deferral-detector
May 11, 2026
Merged

feat: implement Deferral signal detector#2
Joi merged 3 commits into
Joi:mainfrom
mvanhorn:feat/deferral-detector

Conversation

@mvanhorn
Copy link
Copy Markdown
Contributor

Summary

Implements the Deferral signal detector that the README's Signal anatomy section reserves but says "no detector produces them yet."

Why this matters

The Signal enum at crates/jilog-review/src/signal.rs:11-22 already declares Deferral(DeferralSignal), the title formatter in tracker.rs:81-83 already handles it, and Signal::session_id() / Signal::kind() already cover the arm. The only thing missing was the detector and its hook in run_review. This PR fills the gap and updates the README so the documented count matches the implementation.

Changes

  • New detect_deferrals(&[Message], &str) -> Vec<DeferralSignal> mirrors detect_workarounds: a OnceLock<RegexSet> over assistant text with a parallel label table, first-match-wins per message. Patterns cover come back to, defer/deferring, punt on, leave for later, skipping for now, park for now, next session, circle back.
  • detect_workarounds and detect_deferrals are independent. A message like "skipping for now" fires both - the tracker files them under separate titles via the existing signal_title formatter. The "skipping for now" overlap test asserts this is intentional rather than double-counting.
  • DigestReport gains pub deferrals: Vec<DeferralSignal>; run_review populates it.
  • render_digest and write_digest gain a deferrals: &[DeferralSignal] parameter inserted between workarounds and p0_alerts. The existing four render_digest tests pass &[] for the new arg.
  • Digest YAML frontmatter always carries deferrals: N (zero acceptable). The ## Deferrals section is unconditional with _No deferrals detected._ placeholder when empty, matching the existing pattern for Corrections/Errors/Workarounds.
  • The commands/review.rs summary println! and the --json output (from the previous PR) both include the deferrals count. The JSON shape stays at schema_version: 1; this is an additive key.
  • README "Signal anatomy" table gains a Deferral row between Workaround and P0 Alert. Prose at README.md:128 updates from "Four signal types ... Two more (Pattern, Deferral)" to "Five signal types ... One more (Pattern)".

Depends on #1

This branch is built on top of #1 (--json/--since flags), so the diff GitHub renders here includes both commits: #1's 246-line review.rs/query.rs change plus the 215-line deferral work. The two commits are independent in scope; the deferral work just happens to extend the JSON shape #1 introduced.

Easiest review path: merge #1 first, then GitHub will auto-rebase this PR to just the deferral commit. If you prefer the reverse order, the deferral commit can be cherry-picked to main and #1 rebased on top - the file overlap is small.

Testing

  • cargo build --workspace passes
  • cargo test --workspace passes (105 tests total, was ~70 pre-PR)
  • 8 new deferrals_* tests in detectors.rs cover: basic match, first-match-wins, user-role skipped, tool-blocks excluded, no-match returns empty, label emission, the workaround-vs-deferral overlap on "skipping for now", and the digest render path.

The README's Signal anatomy section calls out Pattern and Deferral as
reserved enum variants 'no detector produces them yet.' This wires up
the Deferral detector and renders deferrals through the digest pipeline.

- detect_deferrals mirrors detect_workarounds: OnceLock<RegexSet> with a
  parallel label table, first-match-wins per assistant message. Patterns
  cover come-back-to, deferring, defer, punt, leave-for-later,
  skipping-for-now, park-for-now, next-session, circle-back.
- detect_workarounds and detect_deferrals are independent. A message
  like 'skipping for now' fires both and the tracker files them under
  separate titles.
- DigestReport gains a deferrals: Vec<DeferralSignal> field; run_review
  populates it. render_digest and write_digest gain a deferrals param
  inserted between workarounds and p0_alerts; existing tests pass [].
- Digest YAML frontmatter always carries deferrals: N (zero acceptable).
  ## Deferrals section is unconditional with _No deferrals detected._
  placeholder, mirroring the existing Corrections/Errors/Workarounds
  pattern.
- The CLI summary println in commands/review.rs and the JSON output
  (--json from feat/review-nightly-json-since) both include the count.
- README Signal anatomy table gains a Deferral row between Workaround
  and P0 Alert; the prose drops Deferral from the reserved list.
- 8 deferrals_* tests in detectors.rs plus a digest render test.
@Joi
Copy link
Copy Markdown
Owner

Joi commented May 11, 2026

Thanks again for the first PR! Merging this after #1.

One quick design question for the code itself, not the behavior:

The workaround/deferral overlap on "skipping for now" — I'm happy with both signals firing (the precise categorization is genuinely useful), but right now the only place the "this is intentional" reasoning lives is in the test name. Could you add a sentence to the detect_deferrals doc comment (and maybe to the README's Signal anatomy section) in a follow-up PR noting that deferrals and workarounds CAN overlap intentionally? It'd save the next reader from wondering whether the double-fire is a bug. Not blocking the merge.

Heads up: I landed a change to render_digest's signature this morning (commit 1ab3a16 — adds an issue_index: &HashMap<String, IssueRef> parameter for bidirectional digest↔tracker linking). Both signature changes are additive but touch the same function, so I rebased your branch on top of main to resolve. Apologies for the timing collision — that's on us.

Really appreciate the careful work.

— Joi

@Joi Joi merged commit df4ca8c into Joi:main May 11, 2026
1 check passed
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