RFC Review: AI Agent Sequential Idle Follow-up (BE)
Companion review for
sequential-idle-follow-up.md, produced by therfc-reviewerskill. Valid only for the RFC revision inreviewed_rfc_last_updated.
Executive Summary
- Overall Score:
8.5/10 - Rating:
Agentic-Ready - RFC Type:
backend - Sub-Type:
enhancement - Assessment Confidence:
High - Applied Caps/Gates: none — no category below 7.5; the 9.0+ gate (DMS/DIC/FMC ≥ 8.5) is what holds the score at 8.5, not a cap
- Implementation Readiness Verdict: PROCEED — minor pin-downs (REV-1..3) can be resolved inline during chunks 2/4 without author round-trips
- Report Path:
chatbot/ai-agent-idle-action/rfcs/sequential-idle-follow-up-review.md - RFC Author: Claude (rfc-starter) + Dimas Fauzi Hidayat | Reviewed: 2026-07-05
An agent can implement this RFC without a clarification meeting. Its
distinctive strength is grounding discipline: every anchor is opened with
quoted evidence (Source Verification table), and the review's usual
worst-case failure — an RFC built on a hallucinated codebase — is
structurally absent; the RFC even corrected the PRD's false
"existing worker retry" assumption against retry: false in both worker
files. The biggest remaining softness is a handful of implementation-time
pin-downs left as prose ("suggested: concerns/ or repository", unnamed
repository method for the idempotency query, unenumerated sibling locale
files) — none change the design, all are one-line decisions an agent could
make defensibly, but pinning them removes the last guessing. Nothing must
change before agentic execution; REV-1/REV-2 should be pinned in the same
PR that implements chunks 2 and 4.
Quick Verdict
Why this RFC can be implemented agentically:
- Every design decision is anchored to opened files with line-level evidence; the executor branch, job-args shape, and guards mirror mechanisms already in production (
send_message_after_send.rb:131-158,process_idle_rule_action.rb:56-57). - The three PRD unknowns (pointer storage, idempotency, reset hook) are all Resolved with rejected alternatives and failure paths — zero dangling architecture.
- Execution plan chunks carry exact files, repo-sourced commands, and assertable ACs (e.g. "legacy specs pass unmodified" as the S04 regression gate).
Why this RFC will cause agent guessing or rework:
- Three micro-locations are unpinned (flag-helper file, idempotency repository method, sibling locale files) — the agent would choose sensibly but is technically choosing (REV-1..3).
- ChatService send timeout is "existing client (unchanged)" rather than a number — fine for unchanged behavior, but the FMC spec is inherited, not stated (REV-5, accepted-risk).
Findings Ledger (carry-forward)
| ID | Severity | Finding (one line) | RFC location | Status | First seen | Resolved in | Evidence / fix |
|---|---|---|---|---|---|---|---|
| REV-1 | minor | Flag-helper location is a dangling X-or-Y: "suggested: app/core/use_cases/concerns/ or repository" | §4.C chunk 2 | open | R1 | — | Pin one: recommend app/core/use_cases/concerns/sequential_idle_flag.rb (module, included by the two call sites) — matches the repo's concerns convention |
| REV-2 | minor | Idempotency history-existence check names no repository method/file — agent must invent the query object | Decision 4, §4.C chunk 4 | open | R1 | — | Specify a new Repositories::Histories::HasIdleStepAfter (sibling of the verified HasNewReplyAfter used at process_idle_rule_action.rb:138), args: history_id, room_id, step_index |
| REV-3 | minor | "config/locales/en.yml (+ sibling locales)" — sibling locale files not enumerated or verified; only en.yml:343-347 was opened | §4.C chunk 4 | open | R1 | — | ls config/locales/ during chunk 4; add resolve_ai_idle to every file carrying the closed_reason family (grep verified only en.yml so far) |
| REV-4 | minor | GET list/summary entity embedding of profile unverified — steps exposure on those two endpoints is assumed | §2.4 row 4, §5 LIM-3 | accepted-risk | R1 | — | Already self-tracked as LIM-3 with a chunk-1 spec covering it; acceptable |
| REV-5 | minor | ChatService send timeout inherited ("existing repository client timeout (unchanged)") — no numeric value stated | Detail 3.A | accepted-risk | R1 | — | Behavior unchanged from every existing send; quantifying would require opening the HTTP client config — worth a one-line addition if convenient, not blocking |
Ledger summary: 3 open (0 blocker / 0 major / 3 minor), 0 fixed this cycle, 2 accepted-risk. Open findings promoted to the RFC §5 table by id.
PRD → RFC Traceability Matrix
| PRD Element | RFC Section | Coverage |
|---|---|---|
| IDLE-ADJ-S01 (configure sequence, 8 ACs) | §2.4, Detail 1.C row 1, §4.C chunk 1 | Full |
| IDLE-ADJ-S02 (execute step-by-step, 8 ACs) | Decisions 2/4/5, Detail 2.2, §4.C chunks 3–4 | Full |
| IDLE-ADJ-S03 (reset on reply, 6 ACs) | Decision 3, Detail 2.2 reset sequence, Detail 2.B | Full |
| IDLE-ADJ-S04 (legacy regression, 5 ACs) | Decision 5, §4 Compatibility, chunk-4 AC "legacy specs pass unmodified" | Full |
| PRD §8 CHG-001 (data shape) | Detail 2.1 shape + rules | Full |
| PRD §8 CHG-002 (FE form) | Declared n/a — FE RFC with contract frozen in §2.4 | Full (explicit deferral, not a silent drop) |
| PRD §8 CHG-003 (runtime) | Decision 2–5, Detail 2.2 | Full |
| PRD §9 behaviors 1–5 | PRD-to-Schema table + Details 2.A/2.C | Full |
| PRD §12 five analytics events | §3 Monitoring deviation table | Partial — delivered as history-row fields + Rollbar, DRI-acked (RFC OQ-3 closed); not silent |
| PRD §11/§14 rollout/stage gates | §4 Rollout Strategy | Full |
| PRD OQ 1/2, Assumption 4 | Decisions 2/4/3 | Full — all three explicitly resolved |
Reverse: RESOLVE_AI_IDLE (Decision 7) | PRD has no such requirement | Justified — closes the PRD's own "Auto-resolution via sequence" metric gap (§13); DRI-decided 2026-07-05, PRD changelog v1.2 records it |
| Reverse: bounded re-schedule (Decision 4) | PRD assumed existing retry | Justified — grounding correction, recorded in PRD changelog v1.2 |
Summary: 12 of 13 PRD elements fully covered, 1 partial (acked deviation), 0 missing. 2 RFC additions beyond the PRD, both justified and recorded back into the PRD changelog.
Scorecard (Backend)
| Category | Score | Evidence-Based Rationale |
|---|---|---|
| PRT — PRD Traceability | 9.0 | Bidirectional matrix with composite AC ids; PRD Section Coverage walks §2–§17 + changelog; deferrals explicit, never silent |
| TDC — Technical Decisions | 8.5 | 8 ADR blocks, all with named alternatives + reversibility; honesty escape used correctly (Decision 8 caching). Deduction: REV-1's "concerns/ or repository" is a dangling X-or-Y inside the execution plan |
| DMS — Data Model & Schema | 8.0 | No DDL by design (JSONB, Decision 1); shape spec is DDL-equivalent (types, ranges, requiredness, example, cardinality, PII, retention). Deduction: enforcement is API-layer only — the RFC says so and compensates with the runtime first(3) guard |
| ACV — API Contract & Versioning | 8.0 | Methods/paths/auth/status codes/examples/error catalog complete; all endpoints tagged extended; OpenAPI bundle chunk with repo-sourced validators. Deduction: REV-4 (list/summary exposure assumed) + error envelope referenced to the shared helper rather than pinned JSON |
| DIC — Data Integrity & Consistency | 8.5 | Idempotency key (room, anchor, step_index) with realization mechanism; transaction scope per write path; the one partial-failure window (send-accept vs history-insert) is named and bounded, not hidden. Deduction: REV-2 (query object unnamed) |
| FMC — Failure Mode & Retry Coverage | 8.0 | 3×60s bounded retry with cap semantics, halt behavior, error catalog with codes/status/messages; no-DLQ is an explicit decision with a stated recovery path. Deduction: REV-5 (inherited timeout not quantified) |
| CSS — Concurrency & Scaling | 8.0 | Collision map covers all four races incl. the TOCTOU window (honestly labeled pre-existing); queue topology named; load bounded (≤2 extra jobs/room/cycle) |
| SAS — Security & Authorization | 8.5 | Middlewares + set_role cited with line numbers; per-field validation; injection surface analysis concrete ("no raw SQL in the path"); PaperTrail audit verified at ai_agent.rb:5 |
| MRP — Migration & Rollout Plan | 8.0 | No-migration case airtight (inert-under-flag data); two-layer kill-switch with per-layer rollback; stages with go/no-go. Deduction: provisioning route confirmed only as "follow ai_assist_image_processing mechanism" (DRI-accepted, chunk-6-scoped) |
| OBS — Observability Definition | 7.5 | Honest deviation table mapping all 5 PRD events to real channels; 3am runbook is concrete (4 steps incl. kill-switch). Deduction: no numeric metric emission exists (repo genuinely lacks a bus); alerting is a Rollbar threshold + BI ratio, not a named alert config |
| SBC — Service Boundary & Coupling | 9.0 | Single service, zero new coupling, zero new contracts; the one cross-squad touchpoint (ChatService) reused verbatim and labeled with owner |
| CPA — Pattern Alignment | 9.0 | Patterns table cites an opened reference file per concern; the single deviation (per-job bounded re-schedule) is called out as a deviation and justified in Decision 4 |
| CDG — Compliance & Data Governance | N/A | No compliance trigger — step messages are tenant-authored bot copy in the same JSONB storing identical message fields today (Detail 3.C states the verification) |
Resource & Cost Advisory
- Detail 4.E is reasoned, not boilerplate: ≤2 extra scheduled jobs per idle room per cycle, ~600 bytes/agent config growth, no new infra. No follow-up needed.
Decision Closure Assessment
Decision Index
| # | Decision | Status | Critical Gaps |
|---|---|---|---|
| 1 | Steps in existing JSONB, no migration | Resolved | — |
| 2 | Step pointer on job args | Resolved | — |
| 3 | Reset via existing has_new_reply?, no new listener | Resolved | — |
| 4 | Bounded explicit re-schedule + history-check idempotency | Resolved | REV-2: repository method for the check unnamed (implementation pin, not a design gap) |
| 5 | Steps mode self-contained; legacy byte-identical | Resolved | — |
| 6 | SystemPreferences kill-switch + org-settings targeting | Resolved | REV-1: helper file location unpinned; provisioning route deliberately deferred to chunk 6 (DRI-accepted) |
| 7 | Sequence resolve → closed_reason: RESOLVE_AI_IDLE | Resolved | REV-3: sibling locale files to enumerate at implementation |
| 8 | No caching | Resolved | — (honesty escape, justified by access pattern) |
Aggregate: 8 of 8 decisions Resolved, 0 Partial, 0 Dangling.
Challenge notes (spot-checks on the two highest-risk decisions):
- Decision 2 at 10x load: pointer-on-job-args scales linearly with idle rooms; the failure mode at scale is Sidekiq scheduled-set growth, which the ≤3-step cap bounds at ≤3 jobs per room lifetime per cycle. Holds. Redis-flush loss is documented as pre-existing (LIM-2) — consistent, not hand-waved.
- Decision 4 double-send window: challenged whether the idempotency check truly prevents a duplicate follow_up. It cannot close the send-accept→history-insert crash window, and the RFC says exactly that ("the same partial-failure window every send in this codebase has today"). Honest bound, correct scope call for an ADJUSTMENT RFC.
- Consistency across decisions: Decision 3's ignore-at-fire-time reset is load-bearing for Decision 2's no-cleanup claim; both cite the same guard lines. No conflicts found.
Backend Deep-Dives
All categories scored ≥ 7.5 → abbreviated per rubric optimization rule.
- Data Integrity: RFC's own Detail 2.A is the deep-dive table and it is complete (3 write paths × 7 columns, no "NOT SPECIFIED" cells).
- Concurrency Collision Map: RFC Detail 2.B covers 4 shared resources incl. the retry-vs-reply and config-edit-mid-sequence races; resolution mechanism named per row.
- API Contract Completeness: 4 endpoint rows × request/response/errors/auth/idempotency/examples — complete except REV-4 (list/summary, self-tracked).
- Async Job Spec: RFC Detail 2.C covers both workers × all 9 columns including poison handling (
steps.first(3)+ nil-guards + Rollbar). - Compliance Trigger Check: scanned — no PII/payment/health/cross-border trigger; auth data untouched. CDG stays N/A.
- Mermaid validity: 10/10 blocks parse (validated with
mmdcthis session, twice — after authoring and again after the Decision-7 edits touched blocks 5 and 8). No failing blocks.
Agentic Readiness Deep-Dive
Vague Word Audit
| # | Word/Phrase | Location | Impact | Concrete Replacement |
|---|---|---|---|---|
| 1 | "suggested: app/core/use_cases/concerns/ or repository" | §4.C chunk 2 | agent picks helper home | pin app/core/use_cases/concerns/sequential_idle_flag.rb (REV-1) |
| 2 | "(+ sibling locales)" | §4.C chunk 4 | agent greps locales itself | enumerate after ls config/locales/ (REV-3) |
| 3 | "verify at implementation" | §5 LIM-3 | agent adds a spec | acceptable — the chunk-1 AC already forces the verification |
Total vague words in spec sections: 3 — all minor, all localized to implementation pins.
Dangling Alternatives
| # | Alternatives | Location | Impact |
|---|---|---|---|
| 1 | "console vs data migration" (provisioning) | Detail 4.A / §5 OQ-4 | none on code — chunk 6 is an ops step; DRI accepted deferral |
Total dangling alternatives: 1, non-code, accepted.
Task Decomposition Assessment
| Chunk | Acceptance Criteria | Assessment |
|---|---|---|
| 1 Contract | 422 matrix coverage + persist/echo + flag-OFF rejection | verifiable |
| 2 Flag helper | 4-state truth table spec | verifiable |
| 3 Arm step 0 | worker scheduled with step-0 duration + step_index: 0; legacy contexts unmodified | verifiable |
| 4 Execute/re-arm/retry | 9 assertable behaviors incl. "legacy specs pass unmodified" and "global rule never called (spec asserts)" | verifiable |
| 5 OpenAPI + docs | two validators exit 0, dist regenerated, SESSION-LOG entry | verifiable |
| 6 Flag provisioning | staging end-to-end sequence green | verifiable (ops-gated) |
Strengths
- Source Verification is real, not ceremonial — 15 rows, each with function-level or line-range evidence, including two negative verifications (SyncToAiService grep-zero; no analytics bus) that materially shaped the design (Detail 2.0).
- The RFC corrects its own PRD against the code — the
retry: falsediscovery (§1 Overview, Decision 4) is exactly the failure class RFCs usually ship: an inherited false assumption. Here it became a designed mechanism plus a PRD changelog entry (v1.2). - Regression strategy is structural, not aspirational — "existing specs pass unmodified" as an executable gate for S04 (Detail 1.C row 4, chunk 4 AC), enabled by the Decision 5 branch-don't-rewrite choice.
Biggest Gaps
- REV-1/REV-2 — two implementation locations left to agent judgment (flag-helper file; idempotency query object). Cheap to pin, and pinning them removes the only real "agent would guess" moments (§4.C chunks 2/4).
- REV-3 — Decision 7's locale change is verified only for
en.yml; if anid.yml(or other) carries theclosed_reasonfamily, missing it produces an untranslated reason string in production data (§4.C chunk 4). - OBS ceiling — observability rides history rows + Rollbar because the repo has no metrics bus; acked by the DRI, but it means the <2% failure-rate gate is computed in BI with datamart lag rather than alerting in near-real-time (§3 Monitoring).
Priority Actions
- §4.C chunk 2 (REV-1) — replace "suggested: concerns/ or repository" with the pinned path
app/core/use_cases/concerns/sequential_idle_flag.rb; one-line RFC edit. - Decision 4 / chunk 4 (REV-2) — name the idempotency query object (
Repositories::Histories::HasIdleStepAfter, sibling ofHasNewReplyAfter) with its three args; one-line RFC edit. - §4.C chunk 4 (REV-3) — enumerate the locale files carrying
closed_reason(onels config/locales/+ grep) and list them in the chunk. - (optional) Detail 3.A (REV-5) — quote the actual ChatService client timeout value for completeness.
Implementation Readiness Checklist
Unblocked
- PRD → RFC traceability complete (bidirectional, composite AC ids)
- All technical decisions resolved with alternatives rejected (8/8)
- Failure modes per external interaction + error catalog
- Configuration contract with defaults + provisioners
- Pattern alignment verified (enhancement) — reference file per concern
- Rollout plan with two-layer flag + rollback recipe
- Observability defined (deviation acked by DRI)
- Task decomposition with verifiable ACs per chunk
- Schema shape at DDL-equivalent precision (JSONB contract)
- API contracts with schemas, error taxonomy, examples
- Transaction boundaries + idempotency per write path
- Concurrency collisions mapped with resolutions
- Security: auth/validation/injection/tenancy specified
- Service boundary documented (zero new coupling)
- Compliance N/A verified
Blocked
- (none — REV-1..3 are inline pin-downs, not blockers)
Verdict: Ready to implement.
Task Manifest
RFC specifies its own decomposition (§4.C, 6 chunks) — verified above as complete and verifiable; adopted as-is. No reviewer-proposed substitute needed. Sequence: 1 → 2 → 3 → 4 → 5 → 6, with chunk 4 gated on chunks 2–3 and chunk 6 gated on ops provisioning (OQ-4 route).
Dangling Decisions Log
| # | Decision | Location | Owner | Deadline |
|---|---|---|---|---|
| — | none — 8/8 resolved |
Open Questions
| # | Question | Category | Severity |
|---|---|---|---|
| 1 | REV-1 — which file hosts the flag helper? (recommend pinning app/core/use_cases/concerns/sequential_idle_flag.rb) | TDC | Nice-to-have |
| 2 | REV-2 — confirm the idempotency query object name/args (Repositories::Histories::HasIdleStepAfter) | DIC | Important |
| 3 | REV-3 — which locale files besides en.yml carry the closed_reason family? | TDC | Important |
Evidence Notes
- Detail 2.0 Source Verification — 15 evidence-quoted rows including negative verifications; primary driver of the High confidence rating.
- Decision 4 — the
retry: falsecorrection with a designed replacement mechanism; strongest single section. - §3 Monitoring deviation table — honest capability mapping instead of invented event names; kept OBS at 7.5 rather than penalizing fabrication (there was none).
- §4.C chunk 2 — the one dangling X-or-Y in an otherwise pinned execution plan (REV-1).
- §7 gate — flipped to
yeson dated DRI decisions recorded in the Comment log; consistent with frontmatter and §5 ledger.
Review History
| Cycle | Date | Reviewed RFC revision | Score | Verdict | Findings open → fixed | Notes |
|---|---|---|---|---|---|---|
| R1 | 2026-07-05 | last_updated 2026-07-05 / working tree (uncommitted) | 8.5 | PROCEED | 3 open (all minor), 2 accepted-risk, 0 fixed | Initial review of the post-DRI-decision revision (OQ-2/3/4 closed, Decision 7 added) |