Council of Claudes: orchestrator agent for cross-persona comment dedup

.github/workflows/council_of_claudes.yml

@@ -43,6 +43,9 @@ jobs:
       NELLJERRAM_AGENT_TOKEN: ${{ secrets.NELLJERRAM_AGENT_TOKEN }}
       CASEYDAVENPORT_AGENT_URL: ${{ secrets.CASEYDAVENPORT_AGENT_URL }}
       CASEYDAVENPORT_AGENT_TOKEN: ${{ secrets.CASEYDAVENPORT_AGENT_TOKEN }}
+      # Orchestrator (cross-persona dedup); if unset, all comments are posted unfiltered.
+      ORCHESTRATOR_AGENT_URL: ${{ secrets.ORCHESTRATOR_AGENT_URL }}
+      ORCHESTRATOR_AGENT_TOKEN: ${{ secrets.ORCHESTRATOR_AGENT_TOKEN }}
     steps:
       # pull_request checks out the merge ref by default, so the script comes
       # from the PR's own tree — letting us iterate on the workflow via its PR.

.github/workflows/scripts/council_of_claudes.js

@@ -221,6 +221,90 @@ async function reviewWithAgent({ core, title, url, token, messageText, msgId })
   return null;
 }
 
+// --- Orchestrator: cross-persona deduplication of inline comments ---
+const ORCH_URL_ENV = 'ORCHESTRATOR_AGENT_URL';
+const ORCH_TOKEN_ENV = 'ORCHESTRATOR_AGENT_TOKEN';
+
+// Robustly extract the orchestrator's { clusters: [...] } JSON from its response
+// (tolerates a ```json fence — single- or multi-line — or stray prose). Returns
+// the parsed object or null.
+function parseClusters(text) {
+  if (!text) return null;
+  const tries = [text.trim()];
+  const fence = text.match(/```[a-zA-Z]*\s*([\s\S]*?)```/); // handles single-line fences too
+  if (fence) tries.push(fence[1].trim());
+  const first = text.indexOf('{'), last = text.lastIndexOf('}');
+  if (first !== -1 && last > first) tries.push(text.slice(first, last + 1));
+  for (const t of tries) {
+    try {
+      const obj = JSON.parse(t);
+      if (obj && Array.isArray(obj.clusters)) return obj;
+    } catch { /* try next candidate */ }
+  }
+  return null;
+}
+
+// Pure decision logic: given the inline comments and the orchestrator's parsed
+// clusters, return the SET of ids to drop. No env / network — directly unit
+// testable. Defensive against arbitrary LLM output: honor a cluster only if its
+// survivor is a known id; drop only known duplicate ids; keep-wins (an id that
+// survives anywhere is never dropped); anything not mentioned is implicitly kept.
+function applyClusters({ core, allInline, parsed }) {
+  const known = new Set(allInline.map(c => c.id));
+  const survivors = new Set();
+  const candidates = new Set();
+  for (const cl of parsed.clusters) {
+    // Every field here is LLM-controlled / possibly garbage — guard each access.
+    if (!cl || typeof cl.survivor !== 'string' || !known.has(cl.survivor)) continue;
+    survivors.add(cl.survivor);
+    const dups = Array.isArray(cl.duplicates) ? cl.duplicates.filter(d => known.has(d)) : [];
+    dups.forEach(d => candidates.add(d));
+    if (dups.length) core.info(`  orchestrator: keep ${cl.survivor}, drop [${dups.join(', ')}] — ${safe(cl.reason)}`);
+  }
+  // The prompt says each id appears at most once; if the model listed an id as
+  // both a survivor and a duplicate, keep-wins silently saves it — warn so a
+  // misbehaving orchestrator is visible rather than silently absorbed.
+  const conflicts = [...candidates].filter(id => survivors.has(id));
+  if (conflicts.length) core.warning(`Orchestrator: ${conflicts.length} id(s) listed as both survivor and duplicate (kept via keep-wins): ${conflicts.join(', ')}`);
+  const drop = new Set([...candidates].filter(id => !survivors.has(id)));
+  core.info(`Orchestrator: dropping ${drop.size} duplicate comment(s) of ${allInline.length} inline`);
+  return drop;
+}
+
+// Ask the orchestrator which inline comments are redundant duplicates and return
+// the SET of comment ids to drop. Lossless by construction: returns an empty set
+// (drop nothing → post everything) if the orchestrator is unconfigured, errors,
+// times out, or returns anything we can't safely apply. The try/catch makes the
+// "post everything on failure" guarantee robust even if a callee starts throwing.
+async function orchestrateDedup({ core, allInline }) {
+  const empty = new Set();
+  const url = process.env[ORCH_URL_ENV];
+  const token = process.env[ORCH_TOKEN_ENV];
+  if (!url || !token) {
+    core.info(`Orchestrator not configured (${ORCH_URL_ENV}/${ORCH_TOKEN_ENV} unset) — posting all comments`);
+    return empty;
+  }
+  if (allInline.length < 2) return empty; // nothing to dedupe (need ≥2 comments)
+
+  try {
+    const msgId = `council-orchestrator-${process.env.GITHUB_RUN_ID}`;
+    const payload = JSON.stringify(allInline.map(c =>
+      ({ id: c.id, persona: c.persona, file: c.file, line: c.line, body: c.body })));
+    const messageText =
+      `Here are ${allInline.length} inline review comments from one pull request, as a JSON array. ` +
+      `Identify redundant duplicate groups and return the clusters JSON per your instructions.\n\n${payload}`;
+
+    const out = await reviewWithAgent({ core, title: 'Orchestrator', url, token, messageText, msgId });
+    if (!out) { core.warning('Orchestrator: no response — posting all comments unfiltered'); return empty; }
+    const parsed = parseClusters(out);
+    if (!parsed) { core.warning('Orchestrator: unparseable response — posting all comments unfiltered'); return empty; }
+    return applyClusters({ core, allInline, parsed });
+  } catch (e) {
+    core.warning(`Orchestrator: unexpected error (${safe(e.message)}) — posting all comments unfiltered`);
+    return empty;
+  }
+}
+
 module.exports = async ({ github, context, core }) => {
   const { owner, repo } = context.repo;
   const pr = context.payload.pull_request;
@@ -296,26 +380,47 @@ module.exports = async ({ github, context, core }) => {
   const existingComments = await github.paginate(github.rest.pulls.listReviewComments, { owner, repo, pull_number });
   const repliedTo = new Set(existingComments.map(c => c.in_reply_to_id).filter(id => id != null));
 
-  let summaries = 0;
-  let inlineTotal = 0;
+  // 4a. Parse + classify each persona's findings. Accumulate the anchorable
+  //     inline findings across ALL personas (each with a stable id) so the
+  //     orchestrator can dedupe the whole set; keep summary/folded per persona.
+  const perPersona = [];
+  const allInline = [];
   for (const res of results) {
     if (!res) continue;
     const p = res.persona;
     // Persona badge: a custom avatar image if provided, else the emoji.
     const badge = p.image ? `<img src="${p.image}" width="20" align="top" alt="${p.title}">` : p.emoji;
     const inlineMarker = `<!-- council-of-claudes:inline:${p.key} -->`;
     const { summary, findings } = parseFindings(res.review);
-
-    // Split findings into anchorable vs. unanchorable (folded into summary).
     const folded = [];
     const anchorable = [];
-    for (const f of findings) {
-      if (anchors.get(f.file)?.has(f.line)) anchorable.push(f);
-      else folded.push(f);
-    }
+    findings.forEach((f, i) => {
+      if (anchors.get(f.file)?.has(f.line)) {
+        // Run-scoped ephemeral id: generated and consumed within this single
+        // invocation (sent to the orchestrator, matched against its drop set).
+        // `i` is the index into `findings` (gaps where entries were folded are
+        // harmless — ids only need per-persona uniqueness); never persisted, so
+        // it must not be correlated across runs or against existingComments.
+        const id = `${p.key}-${i}`;
+        anchorable.push({ id, ...f });
+        allInline.push({ id, persona: p.title, file: f.file, line: f.line, body: f.body });
+      } else {
+        folded.push(f); // no valid line anchor → goes to the summary
+      }
+    });
+    perPersona.push({ p, badge, inlineMarker, summary, folded, anchorable });
+  }
 
-    // 4a. Inline: delete this persona's prior comments that have NO replies
-    //     (preserve any thread with discussion), then post the fresh set.
+  // 4b. Orchestrator: dedupe the combined inline set → ids to drop. Lossless on
+  //     any failure / unconfigured (returns empty → post everything).
+  const drop = await orchestrateDedup({ core, allInline });
+
+  // 4c. Post per persona: delete prior no-reply inline, post surviving inline
+  //     (skipping orchestrator-dropped duplicates), then the sticky summary.
+  let summaries = 0;
+  let inlineTotal = 0;
+  let dropped = 0;
+  for (const { p, badge, inlineMarker, summary, folded, anchorable } of perPersona) {
     const priorInline = existingComments.filter(c => (c.body || '').includes(inlineMarker));
     for (const c of priorInline) {
       if (repliedTo.has(c.id)) continue; // keep — has a discussion thread
@@ -327,6 +432,7 @@ module.exports = async ({ github, context, core }) => {
     }
     let inline = 0;
     for (const f of anchorable) {
+      if (drop.has(f.id)) { dropped++; continue; } // redundant — removed by orchestrator
       const body = `${badge} **${p.title}** — ${f.body}\n\n${inlineMarker}`;
       try {
         await github.rest.pulls.createReviewComment({
@@ -341,8 +447,8 @@ module.exports = async ({ github, context, core }) => {
     }
     inlineTotal += inline;
 
-    // 4b. Summary: sticky review, updated in place. Folded findings (no valid
-    //     line anchor) are listed here so nothing is lost.
+    // Summary: sticky review, updated in place. Folded findings (no valid line
+    // anchor) are listed here so nothing is lost.
     const marker = `<!-- council-of-claudes:${p.key} -->`;
     let body =
       `### ${badge} Council of Claudes — ${p.title}\n\n` +
@@ -368,5 +474,5 @@ module.exports = async ({ github, context, core }) => {
       core.warning(`${p.title}: failed to post/update summary (${safe(e.message)})`);
     }
   }
-  core.info(`Done: ${summaries}/${active.length} summaries, ${inlineTotal} inline comment(s) on PR #${pull_number}`);
+  core.info(`Done: ${summaries}/${active.length} summaries, ${inlineTotal} inline posted, ${dropped} duplicate(s) dropped on PR #${pull_number}`);
 };

.github/workflows/scripts/orchestrator.md

@@ -0,0 +1,67 @@
+# Review Orchestrator — Council of Claudes
+
+Paste this as the `systemMessage` when creating the **orchestrator** agent in the
+kagent portal (https://agents.tigera.ai). Model: `claude-opus-4-8`.
+
+This agent is NOT a code reviewer — it does not read the diff or generate review comments. It is a deduplication layer that runs *after* the review personas and decides which of their comments are redundant restatements of each other.
+
+---
+
+You are the **deduplication orchestrator** for a multi-agent code-review system. Several independent reviewer personas have each left inline comments on one pull request. Personas frequently make the **same point** as one another (and sometimes repeat themselves), in different words. Your only job is to find those redundant groups so the system can keep one comment per point and drop the rest.
+
+You do **not** judge whether a comment is correct, valuable, or nit-picky. You do **not** rewrite, summarize, or generate any comment text. You only group genuine duplicates and pick which one to keep.
+
+## Input
+A JSON array of inline review comments, each:
+```json
+{ "id": "<id — opaque per-request token; echo it back exactly>", "persona": "<persona name>", "file": "<path>", "line": <number>, "body": "<markdown>" }
+```
+
+## What counts as a duplicate
+Two (or more) comments are duplicates **only when they make essentially the same actionable point** — the same underlying issue AND the same suggested fix/direction — even if:
+- worded differently, or
+- authored by different personas (or the same persona twice), or
+- anchored a line or two apart.
+
+These are **NOT** duplicates (keep them separate):
+- Same file / function / topic but **different issues** (e.g. two distinct bugs in one function).
+- A **general** observation vs. a **specific instance** of it.
+- The same kind of issue (e.g. "naming") raised about **different** identifiers/locations.
+- Comments that merely *relate* to each other but ask for different things.
+
+When in doubt, **do not group them** — leave them separate. It is much worse to wrongly merge two distinct findings (losing one) than to leave a borderline pair unmerged. Precision over recall.
+
+## Choosing the survivor
+For each duplicate group, choose exactly one **survivor**: the single **clearest, most actionable, best-articulated** comment — the one that would most help the PR author understand and fix the issue.
+Choose on clarity/usefulness alone, **regardless of which persona authored it**. The other comments in the group are the duplicates to drop.
+
+## Output
+Respond with **only** a JSON object (a fenced ```json block is acceptable), in exactly this shape:
+```json
+{
+  "clusters": [
+    { "survivor": "<id>", "duplicates": ["<id>", "<id>"], "reason": "<one-line gist of the shared point>" }
+  ]
+}
+```
+Rules for the output:
+- Include a cluster **only** if it has at least one duplicate (group size ≥ 2). A comment with no duplicate must **not** appear anywhere in the output — anything you don't mention is kept.
+- Use the **exact** `id` values from the input. Never invent ids or return ids that weren't provided.
+- Each id may appear **at most once** across the whole output (as a survivor or a duplicate, not both, and not in two clusters).
+- Never include comment bodies, rewrites, or any prose outside the JSON object.
+- If there are no duplicates at all, return `{ "clusters": [] }`.
+
+## Example
+Input (abbreviated):
+```json
+[
+  {"id":"a1","persona":"Correctness","file":"x.go","line":42,"body":"This `==` is case-sensitive; use strings.EqualFold or the feature silently disables."},
+  {"id":"b2","persona":"Nell","file":"x.go","line":42,"body":"Comparison is case-sensitive — should this use EqualFold?"},
+  {"id":"c3","persona":"Security","file":"y.go","line":10,"body":"Interval has no lower bound; a tiny value could hammer the dataplane."}
+]
+```
+Output:
+```json
+{ "clusters": [ { "survivor": "a1", "duplicates": ["b2"], "reason": "case-sensitive compare should use EqualFold" } ] }
+```
+(`c3` is unique, so it is absent from the output and will be kept.)

hack/council-of-claudes/README.md

@@ -7,6 +7,9 @@ The idea: take a real upstream PR that humans reviewed, reproduce **the exact co
 reviewers first saw**, open it as a PR in this fork so the Council reviews it, then compare the
 Council's feedback to the original human review.
 
+**Design:** the orchestrator (cross-persona dedup) layer and the review-quality decisions are
+documented in [`orchestrator-design.md`](./orchestrator-design.md).
+
 ## Tools
 
 ### `gen-benchmark-pr.sh <upstream-PR-number>`

hack/council-of-claudes/gen-benchmark-pr.sh

@@ -65,8 +65,12 @@ echo "  author: @$author   human top-level review comments: $humans"
 echo "  base  : ${base:0:12}"
 echo "  head  : ${head:0:12}  (earliest human-reviewed commit)"
 
-baseBr="coc-sample-$N-base"
-headBr="coc-sample-$N-head"
+# Optional iteration label (ITER=v2) → parallel duplicate PRs for the same
+# original, so a new run doesn't clobber an earlier labeled baseline. Empty by
+# default (branches stay coc-sample-<N>-base/head).
+SUFFIX="${ITER:+-$ITER}"
+baseBr="coc-sample-$N$SUFFIX-base"
+headBr="coc-sample-$N$SUFFIX-head"
 
 # Resolve the fork point (merge-base) and the as-reviewed diff via the GitHub
 # compare API rather than local git. This works even when the PR was force-pushed
@@ -147,7 +151,7 @@ fi
 
 echo "Opening PR ..."
 gh pr create --repo "$FORK" --base "$baseBr" --head "$headBr" \
-  --title "[sample #$N] $title" \
+  --title "[sample #$N]${ITER:+ [$ITER]} $title" \
   --body "Benchmark sample reproducing the **as-first-reviewed** state of [$UPREPO#$N](https://github.com/$UPREPO/pull/$N) (by @$author).
 
 This PR's diff equals what the human reviewers first saw: fork point \`${fork:0:12}\` → earliest human-reviewed commit \`${head:0:12}\`. The original PR drew **$humans** human top-level review comments — the ground truth to compare the Council's feedback against.