research: reject-with-revision loop (R2)
ci / gates (push) Successful in 6s
ci / rust (push) Failing after 14s
ci / frontend (push) Successful in 28s
ci / e2e (push) Has been skipped
ci / publish (push) Has been skipped

Before: reviewer rejected a publish → audit log flipped, topic stayed
in reviewing, no way to feed the critique back into the run pipeline.
Reviewers with revision notes had to eat them or hand-message the
coordinator.

Now: reject accepts an optional `notes` field. When present:
- Persisted on the research_publish_approvals row (migration 0039).
- Topic flips `reviewing → standby` so the next `start_topic` is legal.
- `start_topic` reads the most recent rejected-approval notes for the
  topic and prepends "PRIOR REVIEW NOTES (address these in this
  revision):\n<notes>\n---" to the coordinator task.

Loop closes through the same run pipeline — no new spawn code path,
which means the reviewer's guidance flows through the same
topology_worker, run_events, outcome-writer chain and lands as a fresh
research_outcomes row (versioned, prior drafts preserved). No notes on
reject = legacy behavior (topic stays in reviewing, publish requests
still allowed).

Migration 0039 adds nullable `notes TEXT` to
research_publish_approvals. `decide()` gains a `notes: Option<&str>`
parameter (only one caller, updated inline). New
`latest_rejection_notes(pool, topic_id)` helper for start_topic.

Frontend:
- rejectPublish(id, notes?) now sends a JSON body when notes are
  provided.
- ResearchCanvas reject button opens an inline form with a textarea +
  Cancel/"Send back for revision" pair. Empty notes → plain reject.
- Button label switches: "Send back for revision" when notes present,
  "Reject without notes" when empty.

Follow-up:
- Notes shown in the review UI on the resulting draft so the next
  reviewer sees what changed.
- Multiple rejection rounds — currently only the LATEST rejection's
  notes surface. Accumulating history is a schema-only tweak.
This commit is contained in:
Omar Sobh
2026-07-09 15:56:16 -07:00
parent 4831033910
commit e3011ed025
5 changed files with 191 additions and 21 deletions
+48 -5
View File
@@ -742,7 +742,7 @@ pub async fn start_topic(
}) })
.collect::<Vec<_>>() .collect::<Vec<_>>()
.join("\n"); .join("\n");
let task = build_coordinator_task( let mut task = build_coordinator_task(
&topic.title, &topic.title,
&topic.outcome_kind, &topic.outcome_kind,
&topic.description, &topic.description,
@@ -750,6 +750,17 @@ pub async fn start_topic(
&roster_lines, &roster_lines,
repo_context.as_ref(), repo_context.as_ref(),
); );
// R2 — reject-with-revision. If the last publish decision was a
// reject with reviewer notes, prepend them to the coordinator task
// as revision guidance. This is what closes the loop: the reviewer's
// critique steers the next iteration through the same run pipeline.
if let Ok(Some(notes)) =
cm_db::repo::research_publish_approvals::latest_rejection_notes(&state.pool, id).await
{
task = format!(
"PRIOR REVIEW NOTES (address these in this revision):\n{notes}\n\n---\n\n{task}"
);
}
let run_id = uuid::Uuid::now_v7(); let run_id = uuid::Uuid::now_v7();
cm_db::repo::topology_runs::enqueue_run_for_research_topic( cm_db::repo::topology_runs::enqueue_run_for_research_topic(
@@ -852,13 +863,18 @@ pub async fn list_pending_publish(
} }
/// Shared body of approve + reject. On approve, transition the topic /// Shared body of approve + reject. On approve, transition the topic
/// `reviewing → publishing` (and set published_at via set_status). On /// `reviewing → publishing → published` and teardown its container. On
/// reject, topic stays put; new requests are allowed. /// reject: audit the decision. If the reject carries revision `notes`,
/// bump the topic `reviewing → standby` so a subsequent `start_topic`
/// spawns a fresh run with the reviewer's guidance folded into the
/// coordinator prompt (R2). Without notes: legacy behavior — topic
/// stays in reviewing, new publish requests are allowed.
async fn decide_publish( async fn decide_publish(
state: AppState, state: AppState,
user: cm_auth::AuthedUser, user: cm_auth::AuthedUser,
id: Uuid, id: Uuid,
approve: bool, approve: bool,
notes: Option<String>,
) -> Result<StatusCode, ApiError> { ) -> Result<StatusCode, ApiError> {
let approval = let approval =
cm_db::repo::research_publish_approvals::get(&state.pool, id, user.workspace_id.as_uuid()) cm_db::repo::research_publish_approvals::get(&state.pool, id, user.workspace_id.as_uuid())
@@ -867,12 +883,17 @@ async fn decide_publish(
if approval.status != "pending" { if approval.status != "pending" {
return Err(ApiError::Conflict); return Err(ApiError::Conflict);
} }
let notes_ref = notes
.as_deref()
.map(str::trim)
.filter(|s| !s.is_empty());
let landed = cm_db::repo::research_publish_approvals::decide( let landed = cm_db::repo::research_publish_approvals::decide(
&state.pool, &state.pool,
id, id,
user.workspace_id.as_uuid(), user.workspace_id.as_uuid(),
user.user_id.as_uuid(), user.user_id.as_uuid(),
approve, approve,
notes_ref,
) )
.await?; .await?;
// Someone else won the race — treat as a no-op success; the topic // Someone else won the race — treat as a no-op success; the topic
@@ -910,24 +931,46 @@ async fn decide_publish(
// The research work is done — tear down the per-topic team // The research work is done — tear down the per-topic team
// container. Artifact-writing already happened during runs. // container. Artifact-writing already happened during runs.
crate::research_container::teardown(approval.topic_id).await; crate::research_container::teardown(approval.topic_id).await;
} else if notes_ref.is_some() {
// Reject-with-revision (R2): flip the topic back to standby so
// the reviewer's guidance takes effect on the next `start_topic`
// via `latest_rejection_notes` in the coordinator prompt.
cm_db::repo::research_topics::set_status(
&state.pool,
approval.topic_id,
user.workspace_id.as_uuid(),
"standby",
)
.await?;
} }
Ok(StatusCode::NO_CONTENT) Ok(StatusCode::NO_CONTENT)
} }
#[derive(serde::Deserialize, Default)]
pub struct RejectPublishRequest {
/// Optional revision guidance. When present + non-empty, decide_publish
/// bumps the topic back to standby and start_topic reads the notes
/// from the approval row to steer the next coordinator prompt.
#[serde(default)]
pub notes: Option<String>,
}
pub async fn approve_publish( pub async fn approve_publish(
State(state): State<AppState>, State(state): State<AppState>,
Authed(user): Authed, Authed(user): Authed,
Path(id): Path<Uuid>, Path(id): Path<Uuid>,
) -> Result<StatusCode, ApiError> { ) -> Result<StatusCode, ApiError> {
decide_publish(state, user, id, true).await decide_publish(state, user, id, true, None).await
} }
pub async fn reject_publish( pub async fn reject_publish(
State(state): State<AppState>, State(state): State<AppState>,
Authed(user): Authed, Authed(user): Authed,
Path(id): Path<Uuid>, Path(id): Path<Uuid>,
body: Option<Json<RejectPublishRequest>>,
) -> Result<StatusCode, ApiError> { ) -> Result<StatusCode, ApiError> {
decide_publish(state, user, id, false).await let notes = body.and_then(|Json(b)| b.notes);
decide_publish(state, user, id, false, notes).await
} }
/// `GET /api/research/:id/artifact` — download the latest outcome as /// `GET /api/research/:id/artifact` — download the latest outcome as
@@ -102,25 +102,56 @@ pub async fn list_pending(
/// Atomically flip a pending row to approved/rejected. Returns whether the /// Atomically flip a pending row to approved/rejected. Returns whether the
/// caller was the one who won the race — false when the row was already /// caller was the one who won the race — false when the row was already
/// decided (idempotent). /// decided (idempotent). Optional `notes` are stashed on the row so a
/// subsequent `start_topic` can pick them up as revision guidance
/// (R2 — reject-with-revision).
pub async fn decide( pub async fn decide(
pool: &PgPool, pool: &PgPool,
id: Uuid, id: Uuid,
workspace_id: Uuid, workspace_id: Uuid,
decided_by: Uuid, decided_by: Uuid,
approve: bool, approve: bool,
notes: Option<&str>,
) -> Result<bool, DbError> { ) -> Result<bool, DbError> {
let new_status = if approve { "approved" } else { "rejected" }; let new_status = if approve { "approved" } else { "rejected" };
let result = sqlx::query!( // Dynamic sqlx::query so the new `notes` column doesn't need a fresh
// .sqlx offline cache entry — the value is bound at runtime.
let result = sqlx::query(
"UPDATE research_publish_approvals "UPDATE research_publish_approvals
SET status = $4, decided_by = $3, decided_at = now() SET status = $4, decided_by = $3, decided_at = now(), notes = $5
WHERE id = $1 AND workspace_id = $2 AND status = 'pending'", WHERE id = $1 AND workspace_id = $2 AND status = 'pending'",
id,
workspace_id,
decided_by,
new_status,
) )
.bind(id)
.bind(workspace_id)
.bind(decided_by)
.bind(new_status)
.bind(notes)
.execute(pool) .execute(pool)
.await?; .await?;
Ok(result.rows_affected() > 0) Ok(result.rows_affected() > 0)
} }
/// Most-recent rejected-approval notes for a topic, or None. Used by
/// `start_topic` to prepend a reviewer's revision guidance to the next
/// coordinator task. Only returns non-empty strings; a rejection with
/// no notes reads the same as no rejection at all.
pub async fn latest_rejection_notes(
pool: &PgPool,
topic_id: Uuid,
) -> Result<Option<String>, DbError> {
use sqlx::Row;
let row: Option<sqlx::postgres::PgRow> = sqlx::query(
"SELECT notes
FROM research_publish_approvals
WHERE topic_id = $1 AND status = 'rejected' AND notes IS NOT NULL
ORDER BY decided_at DESC NULLS LAST
LIMIT 1",
)
.bind(topic_id)
.fetch_optional(pool)
.await?;
Ok(row
.and_then(|r| r.try_get::<Option<String>, _>("notes").ok().flatten())
.map(|s| s.trim().to_string())
.filter(|s| !s.is_empty()))
}
@@ -92,6 +92,8 @@ export function ResearchCanvas({
const [topic, setTopic] = useState<TopicDetail | null>(null); const [topic, setTopic] = useState<TopicDetail | null>(null);
const [pendingApproval, setPendingApproval] = useState<PublishApproval | null>(null); const [pendingApproval, setPendingApproval] = useState<PublishApproval | null>(null);
const [decidingApproval, setDecidingApproval] = useState<"approve" | "reject" | null>(null); const [decidingApproval, setDecidingApproval] = useState<"approve" | "reject" | null>(null);
const [rejectFormOpen, setRejectFormOpen] = useState(false);
const [rejectNotes, setRejectNotes] = useState("");
const [loading, setLoading] = useState(false); const [loading, setLoading] = useState(false);
const [error, setError] = useState<string | null>(null); const [error, setError] = useState<string | null>(null);
const [acting, setActing] = useState(false); const [acting, setActing] = useState(false);
@@ -208,13 +210,15 @@ export function ResearchCanvas({
? pendingApproval ? pendingApproval
: null; : null;
async function decide(kind: "approve" | "reject") { async function decide(kind: "approve" | "reject", notes?: string) {
if (!boundApproval || decidingApproval) return; if (!boundApproval || decidingApproval) return;
setDecidingApproval(kind); setDecidingApproval(kind);
setError(null); setError(null);
try { try {
if (kind === "approve") await approvePublish(boundApproval.id); if (kind === "approve") await approvePublish(boundApproval.id);
else await rejectPublish(boundApproval.id); else await rejectPublish(boundApproval.id, notes);
setRejectFormOpen(false);
setRejectNotes("");
onChanged(); onChanged();
} catch (e) { } catch (e) {
setError(e instanceof Error ? e.message : "decision failed"); setError(e instanceof Error ? e.message : "decision failed");
@@ -580,8 +584,8 @@ export function ResearchCanvas({
</button> </button>
<button <button
type="button" type="button"
onClick={() => decide("reject")} onClick={() => setRejectFormOpen(true)}
disabled={!boundApproval || decidingApproval !== null} disabled={!boundApproval || decidingApproval !== null || rejectFormOpen}
style={{ style={{
padding: "9px 16px", padding: "9px 16px",
borderRadius: 8, borderRadius: 8,
@@ -591,18 +595,92 @@ export function ResearchCanvas({
fontSize: 13, fontSize: 13,
fontWeight: 700, fontWeight: 700,
cursor: cursor:
boundApproval && decidingApproval === null boundApproval && decidingApproval === null && !rejectFormOpen
? "pointer" ? "pointer"
: "default", : "default",
opacity: opacity:
!boundApproval || decidingApproval !== null !boundApproval || decidingApproval !== null || rejectFormOpen
? 0.55 ? 0.55
: 1, : 1,
}} }}
> >
{decidingApproval === "reject" ? "Rejecting…" : "Reject"} Reject
</button> </button>
</div> </div>
{rejectFormOpen ? (
<div
style={{
marginTop: 12,
padding: 12,
borderRadius: 8,
border: "1px solid rgba(255,138,122,.3)",
background: "rgba(255,138,122,.06)",
display: "flex",
flexDirection: "column",
gap: 10,
}}
>
<label style={{ fontSize: 11.5, color: "#cfcfd5", fontWeight: 600 }}>
Revision guidance (optional — send back for another pass)
</label>
<textarea
value={rejectNotes}
onChange={(e) => setRejectNotes(e.target.value)}
placeholder="What should the coordinator address in the next iteration?"
rows={4}
autoFocus
style={{
width: "100%",
resize: "vertical",
borderRadius: 6,
border: "1px solid rgba(255,255,255,.12)",
background: "#101014",
color: "#eaeaee",
fontSize: 12.5,
padding: "8px 10px",
outline: "none",
fontFamily: "inherit",
}}
/>
<div style={{ display: "flex", gap: 8, justifyContent: "flex-end" }}>
<button
type="button"
onClick={() => { setRejectFormOpen(false); setRejectNotes(""); }}
disabled={decidingApproval !== null}
style={{
padding: "7px 14px",
borderRadius: 6,
border: "1px solid rgba(255,255,255,.12)",
background: "transparent",
color: "#cfcfd5",
fontSize: 12,
cursor: "pointer",
}}
>
Cancel
</button>
<button
type="button"
onClick={() => decide("reject", rejectNotes)}
disabled={decidingApproval !== null}
style={{
padding: "7px 14px",
borderRadius: 6,
border: 0,
background: rejectNotes.trim() ? "#ff8a7a" : "rgba(255,138,122,.4)",
color: "#1a0808",
fontSize: 12,
fontWeight: 700,
cursor: decidingApproval === null ? "pointer" : "default",
}}
>
{decidingApproval === "reject"
? (rejectNotes.trim() ? "Sending back…" : "Rejecting…")
: (rejectNotes.trim() ? "Send back for revision" : "Reject without notes")}
</button>
</div>
</div>
) : null}
</div> </div>
) : null} ) : null}
</div> </div>
+6 -2
View File
@@ -138,8 +138,12 @@ export const listPendingApprovals = () =>
export const approvePublish = (id: string) => export const approvePublish = (id: string) =>
api<void>(`/api/research/publish-approvals/${id}/approve`, { method: "POST" }); api<void>(`/api/research/publish-approvals/${id}/approve`, { method: "POST" });
export const rejectPublish = (id: string) => export const rejectPublish = (id: string, notes?: string) =>
api<void>(`/api/research/publish-approvals/${id}/reject`, { method: "POST" }); api<void>(`/api/research/publish-approvals/${id}/reject`, {
method: "POST",
body: notes && notes.trim() ? JSON.stringify({ notes: notes.trim() }) : undefined,
headers: notes && notes.trim() ? { "Content-Type": "application/json" } : undefined,
});
export const wizardRefine = (body: { export const wizardRefine = (body: {
prompt: string; prompt: string;
@@ -0,0 +1,14 @@
-- Adds a nullable `notes` column to research_publish_approvals so a
-- reviewer rejecting a publish can include revision guidance the next
-- coordinator run picks up (R2 — reject-with-revision loop).
--
-- The notes are stashed on the approval row, then `start_topic` reads the
-- most recent rejected approval for the topic and prepends its notes to
-- the coordinator task string as "Prior review notes:". This closes the
-- loop without duplicating the run-spawn code — the reviewer's critique
-- steers the next iteration through the same start_topic path.
--
-- NULL means "reject with no revision guidance" — legacy behavior stays
-- correct (topic stays in reviewing, nothing else fires).
ALTER TABLE research_publish_approvals
ADD COLUMN notes TEXT;