fix(issues): attribute agent-authored comments instead of rendering them as "Board" (#8833)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Task/issue threads render each comment as a chat bubble; the author determines whether it shows as a left-aligned agent bubble (name + icon) or a right-aligned "Board" bubble > - Comments posted by an agent from a local execution environment are written with a non-human author id (`local-board`/system), so they were mis-rendered as blue "Board" bubbles instead of being attributed to the authoring agent > - This misattribution is confusing (it looks like the human board said something an agent actually said) and it can drive false wake/reconciliation behavior on the affected threads > - This pull request adds server-side attribution derivation (lossless run-id join first, then an explicit run-log post marker), persists the derived agent so the read path stops re-scanning run logs, and stops the client from labeling agent-derived comments "Board" > - The benefit is agent comments render as the correct agent, genuine human board comments are never reattributed, and reads get cheaper after a one-time persistence ## Linked Issues or Issue Description <!-- No public GitHub issue — describing the problem in-PR (bug report shape). --> **What happened?** In a task/issue comment thread, comments authored by an agent from a local execution environment are stored with a non-human author id (`local-board`/system). The UI renders these as right-aligned blue "Board" bubbles, implying a human board member authored them. The mislabeling is also a wake/reconciliation hazard: an agent comment that reads as "Board" can look like human board input. **Expected behavior** Such comments should render as the authoring agent (left-aligned bubble with agent name + icon). Genuine human/board comments must continue to render as "Board" and must never be reattributed to an agent. **Steps to reproduce** 1. Have an agent post a comment on an issue from a local execution environment (author id `local-board`). 2. Open the issue comment thread in the UI. 3. Observe the agent's comment rendered as a right-aligned blue "Board" bubble instead of the authoring agent. **Root cause** The read path did not resolve the authoring agent for these comments, and the client fell back to a "Board" label for the `local-board` author. ## What Changed - **Server derivation (`server/src/services/issues.ts`):** - Resolve the authoring agent from the comment's run id first (`createdByRunId`/`derivedCreatedByRunId` → `heartbeatRuns.agentId`) — lossless when present. - Second tier `run_log_comment_post`: read the run log lazily (only for still-unresolved comments) to match the explicit `comment id:` post marker. - **Guard:** never reattribute a comment whose author maps to a genuine user profile. Only the non-human sentinel (`local-board`, which is itself a `user` row) and authors absent from the `user` table are eligible. - Pure timing-overlap tiers are intentionally **not** used (Option A) — an agent comment and a human board comment posted during the same run are indistinguishable rows, so any timing guess risks mislabeling a real human comment. - **Persistence (`packages/db/src/migrations/0126_issue_comment_derived_attribution.sql`, `packages/db/src/schema/issue_comments.ts`):** add stored `derived_*` attribution columns and write the resolved agent back with a single bulk `UPDATE ... FROM (VALUES ...)`, so reads stop recomputing from run logs. Migration is additive (new nullable columns) with a batched, idempotent backfill of the lossless run-id tier over historical rows. - **Types (`packages/shared/src/types/issue.ts`):** expose the persisted attribution fields and the `IssueCommentDerivedAuthorSource` union. - **Client (`ui/src/lib/issue-chat-messages.test.ts`):** the message builder already prefers a resolved agent id (`authorAgentId ?? runAgentId ?? derivedAuthorAgentId`), so once the server persists the derived agent the bubble renders as the agent automatically — no client code change needed. Adds a regression guard confirming a genuine board comment with no derived agent is still rendered as "Board". - **Tests:** derivation + message-building tests, including assertions that genuine board/user comments are **not** reattributed. ## Verification - `cd server && npx vitest run issues-service` — 94 tests pass: run-id resolution, no-attribution on timing overlap alone (Option A), multi-run ambiguity, same-agent multi-run, and the genuine-user guard. Exercises the real persistence path (bulk UPDATE) against the test DB. - `cd ui && npx vitest run issue-chat-messages` — 27 tests pass; client no longer labels agent-derived comments "Board", and a genuine board comment with no derived agent is not re-labeled. - `cd server && npm run typecheck` — passes (exit 0). - Manual: on a thread containing old agent-authored comments, the blue "Board" bubbles render as the authoring agent; a genuine board comment on the same thread still renders as "Board". ## Risks - **Mis-reattributing a genuine board comment made during an agent run** → mitigated by the human-profile guard (only `local-board`/system authors are eligible) and by dropping pure timing tiers (Option A): only the lossless run-id join and the explicit run-log post marker attribute history. - **Backfill volume / run-log reads** → the migration backfill is batched (5000 rows/loop) and results are persisted so reads stop recomputing; the read-path persistence is a single bulk UPDATE rather than per-comment round-trips. Migration adds only nullable columns (no destructive change). - The persistence/backfill has **not** been run against any production database as part of opening this PR. ## Model Used Claude Opus 4.8 (`claude-opus-4-8`), extended thinking, via Claude Code with tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs — related open PRs (#6006 narrow attribution run scan, #4729 attribution roll-up, #7014 reaped-run attribution) address different attribution paths; none fix the `local-board` "Board" bubble rendering this PR targets. Supersedes #8832 (same change; branch renamed to drop an internal ticket id per CONTRIBUTING → Branch Naming) - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
scotttong
GitHub
parent
de837e2683
commit
fb2b760915
|
|
@ -0,0 +1,69 @@
|
|||
ALTER TABLE "issue_comments" ADD COLUMN IF NOT EXISTS "derived_author_agent_id" uuid;--> statement-breakpoint
|
||||
ALTER TABLE "issue_comments" ADD COLUMN IF NOT EXISTS "derived_created_by_run_id" uuid;--> statement-breakpoint
|
||||
ALTER TABLE "issue_comments" ADD COLUMN IF NOT EXISTS "derived_author_source" text;--> statement-breakpoint
|
||||
DO $$ BEGIN
|
||||
IF NOT EXISTS (
|
||||
SELECT 1 FROM "pg_constraint" WHERE "conname" = 'issue_comments_derived_author_agent_id_agents_id_fk'
|
||||
) THEN
|
||||
ALTER TABLE "issue_comments" ADD CONSTRAINT "issue_comments_derived_author_agent_id_agents_id_fk" FOREIGN KEY ("derived_author_agent_id") REFERENCES "public"."agents"("id") ON DELETE set null ON UPDATE no action;
|
||||
END IF;
|
||||
END $$;--> statement-breakpoint
|
||||
DO $$ BEGIN
|
||||
IF NOT EXISTS (
|
||||
SELECT 1 FROM "pg_constraint" WHERE "conname" = 'issue_comments_derived_created_by_run_id_heartbeat_runs_id_fk'
|
||||
) THEN
|
||||
ALTER TABLE "issue_comments" ADD CONSTRAINT "issue_comments_derived_created_by_run_id_heartbeat_runs_id_fk" FOREIGN KEY ("derived_created_by_run_id") REFERENCES "public"."heartbeat_runs"("id") ON DELETE set null ON UPDATE no action;
|
||||
END IF;
|
||||
END $$;--> statement-breakpoint
|
||||
-- Backfill agent attribution for historical non-human ("Board") comments so old
|
||||
-- threads stop rendering as blue board bubbles and the read path stops
|
||||
-- re-scanning run logs. Two SQL-computable tiers, both guarded to NEVER touch a
|
||||
-- comment whose author maps to a genuine user profile. The log-marker tier is
|
||||
-- handled lazily on read (it needs object-storage log bodies).
|
||||
--
|
||||
-- Tier `run_id`: the comment's own authoring run resolves to an agent (lossless).
|
||||
-- Batched to keep lock/WAL footprint bounded on large histories.
|
||||
DO $$
|
||||
DECLARE
|
||||
affected integer;
|
||||
BEGIN
|
||||
LOOP
|
||||
WITH batch AS (
|
||||
SELECT c.id AS comment_id, hr.agent_id, hr.id AS run_id
|
||||
FROM issue_comments c
|
||||
JOIN heartbeat_runs hr ON hr.id = c.created_by_run_id
|
||||
WHERE c.author_agent_id IS NULL
|
||||
AND c.derived_author_agent_id IS NULL
|
||||
AND c.author_user_id IS NOT NULL
|
||||
-- Only the non-human board sentinel or non-`user` authors are
|
||||
-- eligible. `local-board` IS a row in "user" (the implicit board
|
||||
-- admin), so it must be allowed explicitly; genuine signups are not.
|
||||
AND (c.author_user_id = 'local-board'
|
||||
OR NOT EXISTS (SELECT 1 FROM "user" u WHERE u.id = c.author_user_id))
|
||||
LIMIT 5000
|
||||
)
|
||||
UPDATE issue_comments c
|
||||
SET derived_author_agent_id = b.agent_id,
|
||||
derived_created_by_run_id = b.run_id,
|
||||
derived_author_source = 'run_id'
|
||||
FROM batch b
|
||||
WHERE c.id = b.comment_id;
|
||||
GET DIAGNOSTICS affected = ROW_COUNT;
|
||||
EXIT WHEN affected = 0;
|
||||
END LOOP;
|
||||
END $$;--> statement-breakpoint
|
||||
-- Option A: the pure run-window TIMING tiers are intentionally NOT applied.
|
||||
-- Because agents post through the `local-board` subprocess, an agent comment and
|
||||
-- a genuine human board comment are indistinguishable rows, so any timing-overlap
|
||||
-- guess mis-attributes human board comments that merely coincided with an agent
|
||||
-- run (e.g. a human board reply typed while an agent run was in flight).
|
||||
-- Only the lossless `run_id` backfill above (and the read-path `run_log_comment_post`
|
||||
-- tier) attribute history; everything else stays "Board".
|
||||
--
|
||||
-- This statement also reverts any attribution a PRIOR revision of this migration
|
||||
-- persisted via the timing tiers, so re-applying / upgrading is idempotent.
|
||||
UPDATE issue_comments
|
||||
SET derived_author_agent_id = NULL,
|
||||
derived_created_by_run_id = NULL,
|
||||
derived_author_source = NULL
|
||||
WHERE derived_author_source IN ('run_window_unique', 'run_window_agent_unique');
|
||||
|
|
@ -883,6 +883,13 @@
|
|||
"when": 1782440100000,
|
||||
"tag": "0125_environment_custom_image_templates",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 126,
|
||||
"version": "7",
|
||||
"when": 1782526400000,
|
||||
"tag": "0126_issue_comment_derived_attribution",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import type {
|
||||
IssueCommentAuthorType,
|
||||
IssueCommentDerivedAuthorSource,
|
||||
IssueCommentMetadata,
|
||||
IssueCommentPresentation,
|
||||
SourceTrustMetadata,
|
||||
|
|
@ -20,6 +21,13 @@ export const issueComments = pgTable(
|
|||
authorUserId: text("author_user_id"),
|
||||
authorType: text("author_type").$type<IssueCommentAuthorType>(),
|
||||
createdByRunId: uuid("created_by_run_id").references(() => heartbeatRuns.id, { onDelete: "set null" }),
|
||||
// Persisted result of best-effort agent-attribution derivation for comments
|
||||
// authored by a non-human sentinel (e.g. `local-board`). Populated once by a
|
||||
// backfill migration and lazily on read so the load path stops re-scanning
|
||||
// run logs.
|
||||
derivedAuthorAgentId: uuid("derived_author_agent_id").references(() => agents.id, { onDelete: "set null" }),
|
||||
derivedCreatedByRunId: uuid("derived_created_by_run_id").references(() => heartbeatRuns.id, { onDelete: "set null" }),
|
||||
derivedAuthorSource: text("derived_author_source").$type<IssueCommentDerivedAuthorSource>(),
|
||||
body: text("body").notNull(),
|
||||
presentation: jsonb("presentation").$type<IssueCommentPresentation | null>(),
|
||||
metadata: jsonb("metadata").$type<IssueCommentMetadata | null>(),
|
||||
|
|
|
|||
|
|
@ -659,6 +659,7 @@ export type {
|
|||
IssueExecutionStagePrincipal,
|
||||
IssueExecutionDecision,
|
||||
IssueComment,
|
||||
IssueCommentDerivedAuthorSource,
|
||||
IssueCommentMetadata,
|
||||
IssueCommentMetadataSection,
|
||||
IssueCommentMetadataRow,
|
||||
|
|
|
|||
|
|
@ -324,6 +324,7 @@ export type {
|
|||
IssueReviewRequest,
|
||||
IssueExecutionDecision,
|
||||
IssueComment,
|
||||
IssueCommentDerivedAuthorSource,
|
||||
IssueCommentMetadata,
|
||||
IssueCommentMetadataSection,
|
||||
IssueCommentMetadataRow,
|
||||
|
|
|
|||
|
|
@ -603,6 +603,22 @@ export interface Issue {
|
|||
updatedAt: Date;
|
||||
}
|
||||
|
||||
/**
|
||||
* Where a comment's derived (non-stored-author) agent attribution came from,
|
||||
* in descending confidence:
|
||||
* - `run_id`: comment carries a `createdByRunId`/`derivedCreatedByRunId` whose
|
||||
* run resolves directly to an agent (lossless).
|
||||
* - `run_log_comment_post`: a run log within the comment window contains the
|
||||
* `comment id: {id}` post marker (lossless: the run recorded posting it).
|
||||
*
|
||||
* Only lossless signals are used. Pure run-window timing overlap is NOT a
|
||||
* source — it cannot distinguish an agent comment from a human board comment
|
||||
* that coincided with a run (Option A).
|
||||
*/
|
||||
export type IssueCommentDerivedAuthorSource =
|
||||
| "run_id"
|
||||
| "run_log_comment_post";
|
||||
|
||||
export interface IssueComment {
|
||||
id: string;
|
||||
companyId: string;
|
||||
|
|
@ -613,7 +629,7 @@ export interface IssueComment {
|
|||
createdByRunId?: string | null;
|
||||
derivedAuthorAgentId?: string | null;
|
||||
derivedCreatedByRunId?: string | null;
|
||||
derivedAuthorSource?: "run_log_comment_post" | null;
|
||||
derivedAuthorSource?: IssueCommentDerivedAuthorSource | null;
|
||||
body: string;
|
||||
presentation: IssueCommentPresentation | null;
|
||||
metadata: IssueCommentMetadata | null;
|
||||
|
|
|
|||
|
|
@ -94,14 +94,81 @@ describe("deriveIssueCommentRunLogAttribution", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("does not rewrite comments without exact run-log proof", () => {
|
||||
it("resolves directly from the comment's own run id without reading logs", () => {
|
||||
const commentId = randomUUID();
|
||||
const runId = randomUUID();
|
||||
const agentId = randomUUID();
|
||||
|
||||
const derived = deriveIssueCommentRunLogAttribution(
|
||||
[
|
||||
{
|
||||
id: commentId,
|
||||
authorAgentId: null,
|
||||
authorUserId: "local-board",
|
||||
createdByRunId: runId,
|
||||
createdAt: new Date("2026-05-11T18:55:40.090Z"),
|
||||
},
|
||||
],
|
||||
[
|
||||
{
|
||||
runId,
|
||||
agentId,
|
||||
createdAt: new Date("2026-05-11T18:51:56.246Z"),
|
||||
startedAt: new Date("2026-05-11T18:51:56.257Z"),
|
||||
finishedAt: new Date("2026-05-11T18:55:45.600Z"),
|
||||
logContent: "",
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
expect(derived.get(commentId)).toEqual({
|
||||
derivedAuthorAgentId: agentId,
|
||||
derivedCreatedByRunId: runId,
|
||||
derivedAuthorSource: "run_id",
|
||||
});
|
||||
});
|
||||
|
||||
it("does NOT attribute on run-window overlap alone — timing is not a lossless signal (option A)", () => {
|
||||
// A human board comment can land inside an agent's run window; since both are
|
||||
// stored as `local-board`, a timing-only guess would mis-attribute it. So a
|
||||
// single overlapping run with no run-id and no log marker stays unresolved.
|
||||
const commentId = randomUUID();
|
||||
const runId = randomUUID();
|
||||
const agentId = randomUUID();
|
||||
|
||||
const derived = deriveIssueCommentRunLogAttribution(
|
||||
[
|
||||
{
|
||||
id: commentId,
|
||||
authorAgentId: null,
|
||||
authorUserId: "local-board",
|
||||
createdByRunId: null,
|
||||
createdAt: new Date("2026-05-11T18:55:40.090Z"),
|
||||
},
|
||||
],
|
||||
[
|
||||
{
|
||||
runId,
|
||||
agentId,
|
||||
createdAt: new Date("2026-05-11T18:51:56.246Z"),
|
||||
startedAt: new Date("2026-05-11T18:51:56.257Z"),
|
||||
finishedAt: new Date("2026-05-11T18:55:45.600Z"),
|
||||
logContent: "posted results without echoing the comment id",
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
expect(derived.has(commentId)).toBe(false);
|
||||
});
|
||||
|
||||
it("does not guess when multiple agent runs overlap and no log proves the author", () => {
|
||||
const commentId = randomUUID();
|
||||
const derived = deriveIssueCommentRunLogAttribution(
|
||||
[
|
||||
{
|
||||
id: commentId,
|
||||
authorAgentId: null,
|
||||
authorUserId: "user-1",
|
||||
authorUserId: "local-board",
|
||||
createdByRunId: null,
|
||||
createdAt: new Date("2026-05-11T18:55:40.090Z"),
|
||||
},
|
||||
|
|
@ -113,7 +180,81 @@ describe("deriveIssueCommentRunLogAttribution", () => {
|
|||
createdAt: new Date("2026-05-11T18:51:56.246Z"),
|
||||
startedAt: new Date("2026-05-11T18:51:56.257Z"),
|
||||
finishedAt: new Date("2026-05-11T18:55:45.600Z"),
|
||||
logContent: "posted results without echoing the comment id",
|
||||
logContent: "no comment id here",
|
||||
},
|
||||
{
|
||||
runId: randomUUID(),
|
||||
agentId: randomUUID(),
|
||||
createdAt: new Date("2026-05-11T18:54:00.000Z"),
|
||||
startedAt: new Date("2026-05-11T18:54:00.000Z"),
|
||||
finishedAt: new Date("2026-05-11T18:56:00.000Z"),
|
||||
logContent: "also nothing",
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
expect(derived.has(commentId)).toBe(false);
|
||||
});
|
||||
|
||||
it("does NOT attribute on same-agent run-window overlap alone (option A)", () => {
|
||||
// Even when every overlapping run is the same agent, timing alone cannot
|
||||
// prove the comment was the agent's vs a human board comment during the run.
|
||||
const commentId = randomUUID();
|
||||
const agentId = randomUUID();
|
||||
|
||||
const derived = deriveIssueCommentRunLogAttribution(
|
||||
[
|
||||
{
|
||||
id: commentId,
|
||||
authorAgentId: null,
|
||||
authorUserId: "local-board",
|
||||
createdByRunId: null,
|
||||
createdAt: new Date("2026-06-29T17:41:59.916Z"),
|
||||
},
|
||||
],
|
||||
[
|
||||
{
|
||||
runId: randomUUID(),
|
||||
agentId,
|
||||
createdAt: new Date("2026-06-29T17:41:26.116Z"),
|
||||
startedAt: new Date("2026-06-29T17:41:26.116Z"),
|
||||
finishedAt: new Date("2026-06-29T17:46:33.794Z"),
|
||||
logContent: "no comment id here",
|
||||
},
|
||||
{
|
||||
runId: randomUUID(),
|
||||
agentId,
|
||||
createdAt: new Date("2026-06-29T17:40:09.531Z"),
|
||||
startedAt: new Date("2026-06-29T17:40:09.531Z"),
|
||||
finishedAt: new Date("2026-06-29T17:46:33.794Z"),
|
||||
logContent: "also nothing",
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
expect(derived.has(commentId)).toBe(false);
|
||||
});
|
||||
|
||||
it("never reattributes a comment that already has a stored agent author", () => {
|
||||
const commentId = randomUUID();
|
||||
const derived = deriveIssueCommentRunLogAttribution(
|
||||
[
|
||||
{
|
||||
id: commentId,
|
||||
authorAgentId: randomUUID(),
|
||||
authorUserId: null,
|
||||
createdByRunId: null,
|
||||
createdAt: new Date("2026-05-11T18:55:40.090Z"),
|
||||
},
|
||||
],
|
||||
[
|
||||
{
|
||||
runId: randomUUID(),
|
||||
agentId: randomUUID(),
|
||||
createdAt: new Date("2026-05-11T18:51:56.246Z"),
|
||||
startedAt: new Date("2026-05-11T18:51:56.257Z"),
|
||||
finishedAt: new Date("2026-05-11T18:55:45.600Z"),
|
||||
logContent: "",
|
||||
},
|
||||
],
|
||||
);
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import {
|
|||
activityLog,
|
||||
agentWakeupRequests,
|
||||
agents,
|
||||
authUsers,
|
||||
approvals,
|
||||
assets,
|
||||
companies,
|
||||
|
|
@ -37,6 +38,7 @@ import type {
|
|||
AcceptedPlanDecomposition,
|
||||
IssueComment,
|
||||
IssueCommentAuthorType,
|
||||
IssueCommentDerivedAuthorSource,
|
||||
IssueCommentMetadata,
|
||||
IssueCommentPresentation,
|
||||
IssueBlockerAttention,
|
||||
|
|
@ -101,6 +103,11 @@ const ISSUE_LIST_RELATED_QUERY_CHUNK_SIZE = 500;
|
|||
export const MAX_CHILD_ISSUES_CREATED_BY_HELPER = 25;
|
||||
const MAX_CHILD_COMPLETION_SUMMARIES = 20;
|
||||
const CHILD_COMPLETION_SUMMARY_BODY_MAX_CHARS = 500;
|
||||
// Non-human author sentinels that agents post under. These ARE eligible for
|
||||
// agent-attribution derivation even though `local-board` is also materialized
|
||||
// as a row in the `user` table (it is the implicit board admin). Genuine human
|
||||
// users — real signups with their own ids — are never reattributed.
|
||||
const NON_HUMAN_SENTINEL_AUTHOR_USER_IDS = new Set<string>(["local-board"]);
|
||||
const ISSUE_COMMENT_RUN_LOG_DERIVATION_MAX_LOG_BYTES = 2_000_000;
|
||||
const ISSUE_COMMENT_RUN_LOG_DERIVATION_CHUNK_BYTES = 256_000;
|
||||
const ISSUE_COMMENT_RUN_LOG_DERIVATION_END_SLACK_MS = 60_000;
|
||||
|
|
@ -169,32 +176,65 @@ type IssueCommentRunLogAttributionRun = {
|
|||
createdAt: Date | string;
|
||||
startedAt?: Date | string | null;
|
||||
finishedAt?: Date | string | null;
|
||||
// Best-effort run log text. May be empty when logs were not read for a tier
|
||||
// that does not need them (run-id / run-window-unique); only the
|
||||
// `run_log_comment_post` tier consults this.
|
||||
logContent: string;
|
||||
};
|
||||
|
||||
type DerivedIssueCommentAttribution = {
|
||||
derivedAuthorAgentId: string;
|
||||
derivedCreatedByRunId: string;
|
||||
derivedAuthorSource: IssueCommentDerivedAuthorSource;
|
||||
};
|
||||
|
||||
/**
|
||||
* Best-effort agent attribution for comments whose stored author is a non-human
|
||||
* sentinel (e.g. `local-board`). Callers MUST pre-filter `comments` to drop any
|
||||
* comment whose `authorUserId` maps to a genuine user profile so a real board /
|
||||
* user comment is never reattributed.
|
||||
*
|
||||
* Only LOSSLESS signals are used — a comment is reattributed solely when a run
|
||||
* provably authored it. Pure run-window timing overlap is intentionally NOT a
|
||||
* signal: because agents post through the `local-board` subprocess, an agent
|
||||
* comment and a genuine human board comment are indistinguishable rows, so any
|
||||
* timing-based guess mis-attributes human board comments that merely coincided
|
||||
* with an agent run (Option A).
|
||||
*
|
||||
* Tiers, in descending confidence (first match wins per comment):
|
||||
* 1. `run_id` — the comment's own `createdByRunId` resolves to an agent run
|
||||
* (lossless: that run authored the comment).
|
||||
* 2. `run_log_comment_post` — an overlapping run log contains the explicit
|
||||
* `comment id: {id}` post marker (lossless: the run recorded posting it).
|
||||
*/
|
||||
export function deriveIssueCommentRunLogAttribution(
|
||||
comments: readonly IssueCommentRunLogAttributionCandidate[],
|
||||
runs: readonly IssueCommentRunLogAttributionRun[],
|
||||
) {
|
||||
const derivedByCommentId = new Map<string, {
|
||||
derivedAuthorAgentId: string;
|
||||
derivedCreatedByRunId: string;
|
||||
derivedAuthorSource: "run_log_comment_post";
|
||||
}>();
|
||||
const derivedByCommentId = new Map<string, DerivedIssueCommentAttribution>();
|
||||
const runById = new Map(runs.map((run) => [run.runId, run] as const));
|
||||
|
||||
for (const comment of comments) {
|
||||
if (comment.authorAgentId || !comment.authorUserId || comment.createdByRunId) continue;
|
||||
if (comment.authorAgentId || !comment.authorUserId) continue;
|
||||
|
||||
// Tier 1: the comment carries the run that authored it. Lossless even when
|
||||
// the author was recorded as the `local-board` sentinel.
|
||||
if (comment.createdByRunId) {
|
||||
const ownRun = runById.get(comment.createdByRunId);
|
||||
if (ownRun?.agentId) {
|
||||
derivedByCommentId.set(comment.id, {
|
||||
derivedAuthorAgentId: ownRun.agentId,
|
||||
derivedCreatedByRunId: ownRun.runId,
|
||||
derivedAuthorSource: "run_id",
|
||||
});
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
const commentCreatedAtMs = toTimestampMs(comment.createdAt);
|
||||
if (commentCreatedAtMs === null) continue;
|
||||
|
||||
let bestMatch:
|
||||
| {
|
||||
runId: string;
|
||||
agentId: string;
|
||||
distanceMs: number;
|
||||
}
|
||||
| null = null;
|
||||
|
||||
const overlappingRuns: Array<{ run: IssueCommentRunLogAttributionRun; runEndMs: number }> = [];
|
||||
for (const run of runs) {
|
||||
const runStartMs = toTimestampMs(run.startedAt ?? run.createdAt);
|
||||
const runEndMs = toTimestampMs(run.finishedAt ?? run.createdAt);
|
||||
|
|
@ -205,24 +245,30 @@ export function deriveIssueCommentRunLogAttribution(
|
|||
) {
|
||||
continue;
|
||||
}
|
||||
if (!run.logContent.includes(`comment id: ${comment.id}`)) continue;
|
||||
|
||||
const distanceMs = Math.abs(runEndMs - commentCreatedAtMs);
|
||||
if (!bestMatch || distanceMs < bestMatch.distanceMs) {
|
||||
bestMatch = {
|
||||
runId: run.runId,
|
||||
agentId: run.agentId,
|
||||
distanceMs,
|
||||
};
|
||||
}
|
||||
overlappingRuns.push({ run, runEndMs });
|
||||
}
|
||||
|
||||
if (!bestMatch) continue;
|
||||
derivedByCommentId.set(comment.id, {
|
||||
derivedAuthorAgentId: bestMatch.agentId,
|
||||
derivedCreatedByRunId: bestMatch.runId,
|
||||
derivedAuthorSource: "run_log_comment_post",
|
||||
});
|
||||
// Tier 2: an overlapping run log explicitly recorded posting this comment.
|
||||
let bestLogMatch: { runId: string; agentId: string; distanceMs: number } | null = null;
|
||||
for (const { run, runEndMs } of overlappingRuns) {
|
||||
if (!run.logContent.includes(`comment id: ${comment.id}`)) continue;
|
||||
const distanceMs = Math.abs(runEndMs - commentCreatedAtMs);
|
||||
if (!bestLogMatch || distanceMs < bestLogMatch.distanceMs) {
|
||||
bestLogMatch = { runId: run.runId, agentId: run.agentId, distanceMs };
|
||||
}
|
||||
}
|
||||
if (bestLogMatch) {
|
||||
derivedByCommentId.set(comment.id, {
|
||||
derivedAuthorAgentId: bestLogMatch.agentId,
|
||||
derivedCreatedByRunId: bestLogMatch.runId,
|
||||
derivedAuthorSource: "run_log_comment_post",
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
// No lossless signal — leave unresolved. A pure run-window timing overlap is
|
||||
// deliberately NOT enough to reattribute (it cannot tell an agent comment
|
||||
// from a human board comment that happened during the run).
|
||||
}
|
||||
|
||||
return derivedByCommentId;
|
||||
|
|
@ -3442,6 +3488,39 @@ export function issueService(db: Db) {
|
|||
return content;
|
||||
}
|
||||
|
||||
// Persist a resolved attribution so subsequent reads stop re-scanning run
|
||||
// logs (and old "Board" threads stay fixed durably). Best-effort: a write
|
||||
// failure must never break the read path. The `IS NULL` guard keeps this
|
||||
// idempotent and avoids clobbering a value another reader just stored.
|
||||
async function persistDerivedIssueCommentAttribution(
|
||||
derivedByCommentId: ReadonlyMap<string, DerivedIssueCommentAttribution>,
|
||||
) {
|
||||
if (derivedByCommentId.size === 0) return;
|
||||
// One bulk `UPDATE ... FROM (VALUES ...)` so the read path is never blocked
|
||||
// on N sequential round-trips for a large legacy thread. The `IS NULL` guard
|
||||
// keeps this idempotent and avoids clobbering a value another reader just
|
||||
// stored. Best-effort: a write failure must never break the read path.
|
||||
const rows = [...derivedByCommentId].map(
|
||||
([commentId, derived]) =>
|
||||
sql`(${commentId}::uuid, ${derived.derivedAuthorAgentId}::uuid, ${derived.derivedCreatedByRunId}::uuid, ${derived.derivedAuthorSource}::text)`,
|
||||
);
|
||||
try {
|
||||
await db.execute(sql`
|
||||
UPDATE ${issueComments} AS c
|
||||
SET derived_author_agent_id = v.agent_id,
|
||||
derived_created_by_run_id = v.run_id,
|
||||
derived_author_source = v.source
|
||||
FROM (VALUES ${sql.join(rows, sql`, `)}) AS v(comment_id, agent_id, run_id, source)
|
||||
WHERE c.id = v.comment_id AND c.derived_author_agent_id IS NULL
|
||||
`);
|
||||
} catch (err) {
|
||||
logger.warn(
|
||||
{ err, commentIds: [...derivedByCommentId.keys()] },
|
||||
"failed to persist derived issue-comment attribution",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
async function enrichCommentsWithDerivedAgentAttribution<
|
||||
T extends {
|
||||
id: string;
|
||||
|
|
@ -3450,20 +3529,55 @@ export function issueService(db: Db) {
|
|||
authorAgentId?: string | null;
|
||||
authorUserId?: string | null;
|
||||
createdByRunId?: string | null;
|
||||
derivedAuthorAgentId?: string | null;
|
||||
createdAt: Date | string;
|
||||
},
|
||||
>(comments: readonly T[]) {
|
||||
const candidates = comments.filter((comment) =>
|
||||
// Candidates: a non-human author, no stored agent, and not already resolved
|
||||
// by a previous read / the backfill migration.
|
||||
const preliminary = comments.filter((comment) =>
|
||||
!comment.authorAgentId
|
||||
&& !!comment.authorUserId
|
||||
&& !comment.createdByRunId,
|
||||
&& !comment.derivedAuthorAgentId,
|
||||
);
|
||||
if (candidates.length === 0) return comments;
|
||||
if (preliminary.length === 0) return comments;
|
||||
|
||||
const companyId = comments[0]?.companyId ?? null;
|
||||
const issueId = comments[0]?.issueId ?? null;
|
||||
if (!companyId || !issueId) return comments;
|
||||
|
||||
// Guard: never reattribute a comment whose author maps to a genuine user
|
||||
// profile. Only the non-human sentinels agents post under (e.g.
|
||||
// `local-board`) are eligible — even though `local-board` is itself a row in
|
||||
// the `user` table, so a plain "exists in user table" check would wrongly
|
||||
// exclude every mis-attributed agent comment.
|
||||
const nonSentinelAuthorUserIds = [
|
||||
...new Set(
|
||||
preliminary
|
||||
.map((comment) => comment.authorUserId)
|
||||
.filter((id): id is string => !!id && !NON_HUMAN_SENTINEL_AUTHOR_USER_IDS.has(id)),
|
||||
),
|
||||
];
|
||||
const genuineUserIds = nonSentinelAuthorUserIds.length
|
||||
? new Set(
|
||||
(
|
||||
await db
|
||||
.select({ id: authUsers.id })
|
||||
.from(authUsers)
|
||||
.where(inArray(authUsers.id, nonSentinelAuthorUserIds))
|
||||
).map((row) => row.id),
|
||||
)
|
||||
: new Set<string>();
|
||||
// `preliminary` already guarantees a truthy `authorUserId`, so only the two
|
||||
// "not a genuine user" arms are live: the explicit non-human sentinel, or an
|
||||
// author id absent from the `user` table.
|
||||
const candidates = preliminary.filter(
|
||||
(comment) =>
|
||||
NON_HUMAN_SENTINEL_AUTHOR_USER_IDS.has(comment.authorUserId!)
|
||||
|| !genuineUserIds.has(comment.authorUserId!),
|
||||
);
|
||||
if (candidates.length === 0) return comments;
|
||||
|
||||
const minCommentCreatedAtMs = candidates.reduce<number | null>((min, comment) => {
|
||||
const timestamp = toTimestampMs(comment.createdAt);
|
||||
if (timestamp === null) return min;
|
||||
|
|
@ -3481,6 +3595,13 @@ export function issueService(db: Db) {
|
|||
maxCommentCreatedAtMs + ISSUE_COMMENT_RUN_LOG_DERIVATION_END_SLACK_MS,
|
||||
).toISOString();
|
||||
|
||||
// The runs the comments' own `createdByRunId` point at — fetched
|
||||
// unconditionally so the lossless run-id tier resolves even when a run is
|
||||
// not otherwise associated with the issue.
|
||||
const ownRunIds = [
|
||||
...new Set(candidates.map((comment) => comment.createdByRunId).filter((id): id is string => !!id)),
|
||||
];
|
||||
|
||||
const runs = await db
|
||||
.select({
|
||||
runId: heartbeatRuns.id,
|
||||
|
|
@ -3497,36 +3618,82 @@ export function issueService(db: Db) {
|
|||
and(
|
||||
eq(heartbeatRuns.companyId, companyId),
|
||||
or(
|
||||
sql`${heartbeatRuns.contextSnapshot} ->> 'issueId' = ${issueId}`,
|
||||
sql`exists (
|
||||
select 1
|
||||
from ${activityLog}
|
||||
where ${activityLog.companyId} = ${companyId}
|
||||
and ${activityLog.entityType} = 'issue'
|
||||
and ${activityLog.entityId} = ${issueId}
|
||||
and ${activityLog.runId} = ${heartbeatRuns.id}
|
||||
)`,
|
||||
and(
|
||||
or(
|
||||
sql`${heartbeatRuns.contextSnapshot} ->> 'issueId' = ${issueId}`,
|
||||
sql`exists (
|
||||
select 1
|
||||
from ${activityLog}
|
||||
where ${activityLog.companyId} = ${companyId}
|
||||
and ${activityLog.entityType} = 'issue'
|
||||
and ${activityLog.entityId} = ${issueId}
|
||||
and ${activityLog.runId} = ${heartbeatRuns.id}
|
||||
)`,
|
||||
),
|
||||
sql`coalesce(${heartbeatRuns.finishedAt}, ${heartbeatRuns.createdAt}) >= ${minCommentCreatedAt}::timestamptz`,
|
||||
sql`coalesce(${heartbeatRuns.startedAt}, ${heartbeatRuns.createdAt}) <= ${maxCommentCreatedAt}::timestamptz`,
|
||||
),
|
||||
ownRunIds.length > 0 ? inArray(heartbeatRuns.id, ownRunIds) : sql`false`,
|
||||
),
|
||||
sql`coalesce(${heartbeatRuns.finishedAt}, ${heartbeatRuns.createdAt}) >= ${minCommentCreatedAt}::timestamptz`,
|
||||
sql`coalesce(${heartbeatRuns.startedAt}, ${heartbeatRuns.createdAt}) <= ${maxCommentCreatedAt}::timestamptz`,
|
||||
),
|
||||
)
|
||||
.orderBy(desc(heartbeatRuns.createdAt));
|
||||
|
||||
if (runs.length === 0) return comments;
|
||||
|
||||
const runsWithLogs: Array<(typeof runs)[number] & { logContent: string }> = [];
|
||||
for (let index = 0; index < runs.length; index += ISSUE_COMMENT_RUN_LOG_DERIVATION_MAX_PARALLEL_READS) {
|
||||
const batch = runs.slice(index, index + ISSUE_COMMENT_RUN_LOG_DERIVATION_MAX_PARALLEL_READS);
|
||||
const batchWithLogs = await Promise.all(batch.map(async (run) => ({
|
||||
...run,
|
||||
logContent: await readRunLogText(run),
|
||||
})));
|
||||
runsWithLogs.push(...batchWithLogs);
|
||||
// Pass 1: resolve the run-id tier, which never reads log bodies. Most
|
||||
// comments resolve here, so we avoid object-storage reads entirely.
|
||||
const runsWithoutLogs = runs.map((run) => ({ ...run, logContent: "" }));
|
||||
const derivedByCommentId = new Map<string, DerivedIssueCommentAttribution>(
|
||||
deriveIssueCommentRunLogAttribution(candidates, runsWithoutLogs),
|
||||
);
|
||||
|
||||
// Pass 2: for comments still unresolved after the run-id tier, read the logs
|
||||
// of any run whose window overlaps such a comment, to look for the explicit
|
||||
// `comment id:` post marker. The marker is a lossless signal regardless of
|
||||
// how many runs overlap, so we do not short-circuit on the single-run case.
|
||||
const unresolved = candidates.filter((comment) => !derivedByCommentId.has(comment.id));
|
||||
if (unresolved.length > 0) {
|
||||
const runIdsToRead = new Set<string>();
|
||||
for (const run of runs) {
|
||||
const runStartMs = toTimestampMs(run.startedAt ?? run.createdAt);
|
||||
const runEndMs = toTimestampMs(run.finishedAt ?? run.createdAt);
|
||||
if (runStartMs === null || runEndMs === null) continue;
|
||||
for (const comment of unresolved) {
|
||||
const commentCreatedAtMs = toTimestampMs(comment.createdAt);
|
||||
if (commentCreatedAtMs === null) continue;
|
||||
if (
|
||||
commentCreatedAtMs >= runStartMs
|
||||
&& commentCreatedAtMs <= runEndMs + ISSUE_COMMENT_RUN_LOG_DERIVATION_END_SLACK_MS
|
||||
) {
|
||||
runIdsToRead.add(run.runId);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (runIdsToRead.size > 0) {
|
||||
const runsToRead = runs.filter((run) => runIdsToRead.has(run.runId));
|
||||
const logByRunId = new Map<string, string>();
|
||||
for (let index = 0; index < runsToRead.length; index += ISSUE_COMMENT_RUN_LOG_DERIVATION_MAX_PARALLEL_READS) {
|
||||
const batch = runsToRead.slice(index, index + ISSUE_COMMENT_RUN_LOG_DERIVATION_MAX_PARALLEL_READS);
|
||||
await Promise.all(
|
||||
batch.map(async (run) => {
|
||||
logByRunId.set(run.runId, await readRunLogText(run));
|
||||
}),
|
||||
);
|
||||
}
|
||||
const runsWithLogs = runs.map((run) => ({ ...run, logContent: logByRunId.get(run.runId) ?? "" }));
|
||||
for (const [commentId, derived] of deriveIssueCommentRunLogAttribution(unresolved, runsWithLogs)) {
|
||||
derivedByCommentId.set(commentId, derived);
|
||||
}
|
||||
}
|
||||
}
|
||||
const derivedByCommentId = deriveIssueCommentRunLogAttribution(candidates, runsWithLogs);
|
||||
|
||||
if (derivedByCommentId.size === 0) return comments;
|
||||
|
||||
await persistDerivedIssueCommentAttribution(derivedByCommentId);
|
||||
|
||||
return comments.map((comment) => {
|
||||
const derived = derivedByCommentId.get(comment.id);
|
||||
return derived ? { ...comment, ...derived } : comment;
|
||||
|
|
|
|||
|
|
@ -462,6 +462,40 @@ describe("buildIssueChatMessages", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("does not reattribute a genuine board/user comment that has no derived agent", () => {
|
||||
const agentMap = new Map<string, Agent>([["agent-1", createAgent("agent-1", "Claude")]]);
|
||||
const messages = buildIssueChatMessages({
|
||||
comments: [
|
||||
createComment({
|
||||
authorUserId: "local-board",
|
||||
authorType: "user",
|
||||
// No agent ever resolved for this comment — a real board action.
|
||||
derivedAuthorAgentId: null,
|
||||
derivedCreatedByRunId: null,
|
||||
runId: null,
|
||||
runAgentId: null,
|
||||
}),
|
||||
],
|
||||
timelineEvents: [],
|
||||
linkedRuns: [],
|
||||
liveRuns: [],
|
||||
agentMap,
|
||||
currentUserId: "user-1",
|
||||
userLabelMap: new Map([["local-board", "Board"]]),
|
||||
});
|
||||
|
||||
expect(messages[0]).toMatchObject({
|
||||
role: "user",
|
||||
metadata: {
|
||||
custom: {
|
||||
authorType: "user",
|
||||
authorAgentId: null,
|
||||
authorUserId: "local-board",
|
||||
},
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("renders a comment as agent-authored when runAgentId is set from activity log", () => {
|
||||
const agentMap = new Map<string, Agent>([["agent-1", createAgent("agent-1", "Claude")]]);
|
||||
const messages = buildIssueChatMessages({
|
||||
|
|
|
|||
Loading…
Reference in New Issue