Conversation
…trade
1. Duplicated "Hello, I'm Serbero..." greeting in clarification messages.
The runtime's `draft_and_send_initial_message` already prefixes a
one-line self-introduction at session open, but the model was also
producing the same greeting inside its `*_clarification` text — both
`phase3-message-templates.md` instructed it to and
`phase3-system.md` told it to "always identify yourself", which it
was reasonably interpreting as "greet on every message." Result:
the greeting appeared twice in a single chat message.
Fix at the prompt layer: drop the greeting from the "First
Clarifying Question" template (the runtime owns it), strip the
"Thank you for your response" preamble from the follow-up template,
narrow the system-prompt identity rule to "answer truthfully if
asked" instead of "always identify yourself", and add an explicit
no-greeting / no-self-introduction hard rule to the classifier
prompt's clarifying-question section.
2. Cooperative self-resolution template wording presumed the recipient
gave the update.
The `[en]/[es]/[pt]` sections opened with "Thanks for the update" /
"Gracias por la actualización" / "Obrigado pela atualização", which
is correct for the party who replied but jarring for the
counterparty who only said "hola". Reworded to a neutral lead-in
("From what each of you has shared, ...") that applies symmetrically
to both parties. Banned-keyword audit and opt-in invariants are
unaffected.
3. PANIC: `set_session_state: illegal transition summary_delivered ->
superseded_by_human`.
Regression introduced by the carve-out commit that added the EXISTS
clause for post-invitation `summary_delivered` sessions to
`latest_open_session_for`. The dispute_resolved handler at
`src/handlers/dispute_resolved.rs` has two distinct paths: lines
156-271 walk live sessions through `SupersededByHuman → Closed`,
and lines 286-322 walk `summary_delivered` sessions through the
legal direct `summary_delivered → Closed` transition. The
carve-out broke that contract by surfacing post-invitation
summary_delivered rows on the SupersededByHuman path, where the
debug-asserted state-machine guard panics.
Reverted only the `latest_open_session_for` carve-out; left the
`list_live_sessions` carve-out in place because the ingest tick
still needs to observe a later party reply on a post-invitation
session for the `PartyRequestedHuman` opt-in. Updated doc
comments on both functions to make the intentional divergence
explicit, and rewrote the inline comment in `mediation::mod.rs`
that described the old (uniform-exclusion) behaviour.
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR updates prompt templates and mediation logic documentation to refine clarification-question formatting policies (preventing duplicate greetings and sign-offs), clarify session eligibility semantics for summary-delivered states, and standardize self-resolution template phrasing across locales. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/db/mediation.rs (1)
427-435: Add a regression test that locks the intentional helper divergence.Given this bug just regressed, it’s worth pinning a test that verifies:
list_live_sessionsincludessummary_deliveredonly withself_resolution_offered, andlatest_open_session_forstill returnsNonefor that same row.That will prevent accidental reintroduction of the panic path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/db/mediation.rs` around lines 427 - 435, Add a regression test around mediation session filtering to lock the behavior that caused the regression: create a test that inserts a session row with state = "summary_delivered" and self_resolution_offered = true and assert that list_live_sessions returns that session, then assert that latest_open_session_for returns None for that same row; conversely insert a "summary_delivered" row with self_resolution_offered = false and assert it is excluded from list_live_sessions. Target your test to call the existing functions list_live_sessions and latest_open_session_for (and use the same DB setup helpers used by other mediation tests) so it fails if the SQL filtering in list_live_sessions or latest_open_session_for regresses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@prompts/phase3-classification.md`:
- Around line 86-94: The current guidance block "Do NOT begin the question with
a greeting..." contains a stale reference to an “always identify yourself”
system rule; remove that sentence and rephrase the block to reflect the runtime
identity disclosure policy (identity is provided once by the runtime in the
first message), e.g., state that subsequent clarification rounds must open
directly with the question because identity is already provided by the runtime's
initial message, and ensure the prohibition on repeated greetings/sign-offs
remains clear (update the block that starts "Do NOT begin the question with a
greeting..." and remove the phrase "always identify yourself").
---
Nitpick comments:
In `@src/db/mediation.rs`:
- Around line 427-435: Add a regression test around mediation session filtering
to lock the behavior that caused the regression: create a test that inserts a
session row with state = "summary_delivered" and self_resolution_offered = true
and assert that list_live_sessions returns that session, then assert that
latest_open_session_for returns None for that same row; conversely insert a
"summary_delivered" row with self_resolution_offered = false and assert it is
excluded from list_live_sessions. Target your test to call the existing
functions list_live_sessions and latest_open_session_for (and use the same DB
setup helpers used by other mediation tests) so it fails if the SQL filtering in
list_live_sessions or latest_open_session_for regresses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 724ae1aa-5c9f-41dc-9d42-00655995e02e
📒 Files selected for processing (6)
prompts/phase3-classification.mdprompts/phase3-message-templates.mdprompts/phase3-self-resolution.mdprompts/phase3-system.mdsrc/db/mediation.rssrc/mediation/mod.rs
… divergence Two follow-ups against the previous commit, raised in the PR review: 1. `prompts/phase3-classification.md`: the new no-greeting hard rule pointed at the system prompt's "always identify yourself" rule, but that phrase was rewritten in the same commit (to "answer truthfully if asked") so the cross-reference dangled. Reworded the block to point at the runtime owner of the introduction (`mediation::draft_and_send_initial_message`) directly. 2. `src/db/mediation.rs`: added a regression test that locks the intentional divergence between `list_live_sessions` (post-invitation `summary_delivered` rows STAY visible so the ingest tick can observe a later party reply for `PartyRequestedHuman`) and `latest_open_session_for` (post-invitation `summary_delivered` rows STAY invisible so the dispute_resolved handler closes them via the legal direct `summary_delivered → closed` transition instead of the illegal `SupersededByHuman` walk). A change that re-aligns the two filters in either direction will now fail this test.
…ipts
Observed 2026-04-28: a freshly-opened Alice/Bob dispute (event
6380cdd2…, dispute 4c64e6ab-…) stayed at `initiated` because the
opening classify+take call returned `Escalate(LowConfidence)`
before any chat-transport step. Mostro never saw Serbero take the
dispute, so the dispute lifecycle did not advance. Log signature:
classify_for_start: rationale persisted dispute-scoped
classification=unclear confidence=0.12
decision=Escalate(LowConfidence)
opening-call escalate: dispute-scoped handoff recorded, no take
trigger=low_confidence
The round-0 bypass in `policy::classify_to_decision` accepts
low-confidence classifications iff `suggested_action =
ask_clarification`. With confidence 0.12 and decision LowConfidence
the bypass did not engage, so the model returned
`suggested_action ∈ { summarize, escalate }` instead of
`ask_clarification`. Two contributing causes, both fixed here:
1. **Blank transcript section.** With `transcript = ""` the user
prompt rendered "## Transcript\n\n## Output contract", which the
`gpt-5.4-mini` router model read as "transcript missing" rather
than "transcript empty because no replies yet". The model fell
back to a defensive `summarize | escalate`. Fix: emit a literal
`(no replies yet — round 0)` marker when `transcript.is_empty()`,
so the round-0 signal is unambiguous in the prompt body.
2. **Hard-rule escape hatch interpreted too broadly.** The
classifier prompt's "If you cannot produce a useful question
for one side, pick a different `suggested_action` (`summarize`
or `escalate`)" rule had no round-0 carve-out. With an empty
transcript a literal reading is "I have no information about
this dispute, so I cannot produce a useful question for either
side, so I should escalate." Fix: added an explicit Round-0
contract section to `prompts/phase3-classification.md` stating
that round 0 MUST return `ask_clarification` with both opening
questions, and narrowed the escape-hatch wording to
`round_count >= 1` — round 0's job is to start the chat, not
classify or escalate.
Pre-fix the "First Clarifying Question" template carried the
"Hello, I'm Serbero, …" greeting, which apparently anchored the
model toward `ask_clarification` on round 0 even without an explicit
contract section. Removing that greeting (necessary to fix the
duplicated-greeting bug) exposed this latent fragility.
Lib + integration tests pass; no schema change.
This was referenced Apr 29, 2026
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
Three production bugs surfaced in the 2026-04-27 Alice/Bob test trade after PR #48 (cooperative self-resolution) shipped. Fixes are isolated to prompt files plus one DB-helper revert; no schema change.
1. Duplicated greeting inside a single chat message
mediation::draft_and_send_initial_messagehard-prefixes "Hello, I'm Serbero, an automated mediation assistant…" on the first outbound. The model was also emitting the same line at the start ofbuyer_clarification/seller_clarificationbecauseprompts/phase3-message-templates.mdtold it to andphase3-system.md's "always identify yourself" rule reinforced it. Result: greeting appeared twice in Alice's chat (screenshot in the production report).phase3-message-templates.md: removed the greeting from "First Clarifying Question" — runtime owns the introduction; stripped the "Thank you for your response" preamble from "Follow-Up Clarification".phase3-system.md: narrowed "always identify yourself" to "answer truthfully if asked" plus an explicit prohibition on prefixing every clarification with a self-introduction.phase3-classification.md: new hard rule under "Clarifying Questions (per-party)" forbidding greetings, self-introductions, and sign-offs in either*_clarificationfield.2. Cooperative invitation presumed the recipient gave the update
prompts/phase3-self-resolution.mdopened each language section with "Thanks for the update" / "Gracias por la actualización" / "Obrigado pela atualização", correct for the party who just replied but nonsensical for a counterparty who only said "hola". Reworded to a neutral lead-in ("From what each of you has shared, …" / "Por lo que cada uno ha compartido, …" / "Pelo que cada um compartilhou, …") that applies symmetrically. Banned-keyword audit and opt-in invariants are unaffected;tests/phase3_self_resolution_template_audit.rscontinues to pass.3. Daemon panic:
summary_delivered → superseded_by_humanillegalAfter Alice released funds, the daemon panicked with:
This is a regression from the earlier carve-out that added the
self_resolution_offeredEXISTS clause to bothlist_live_sessionsandlatest_open_session_for. The dispute_resolved handler atsrc/handlers/dispute_resolved.rshas two distinct paths:SupersededByHuman → Closed.summary_deliveredsessions through the legal directsummary_delivered → Closedtransition.Surfacing post-invitation
summary_deliveredrows on the SupersededByHuman path tripped the debug-asserted state-machine guard.Fix: reverted only the
latest_open_session_forcarve-out; left thelist_live_sessionscarve-out in place because the ingest tick still needs to observe a later party reply on a post-invitation session for thePartyRequestedHumanopt-in. Doc comments on both functions now spell out the intentional divergence, and the inline comment inmediation::mod.rswas rewritten to match.Test plan
cargo test— 295 lib + integration tests pass.cargo clippy --all-targets -- -D warnings— clean.cargo fmt --all -- --check— clean.summary_delivered → closedpath; no panic.Summary by CodeRabbit
Bug Fixes
Documentation