Skip to content

fix(chat): restrict grounding to public summaries #5106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JSONbored wants to merge 1 commit into
base: main
Choose a base branch
from
+23 −11
Open

fix(chat): restrict grounding to public summaries #5106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions src/services/ai-chat-qa.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ const CHAT_QA_SYSTEM_PROMPT =
"question, say so plainly and suggest running `@gittensory preflight` or `@gittensory blockers`.";

// Private decision-pack blocker codes and boundary terms are redacted (not thrown on) before the grounding
// bundle is ever put in a prompt -- publicSafeSummary is already public-safe, but raw `blockedBy`/`why` can
// carry these. Mirrors github/commands.ts's publicBlockerDetail redaction intent without importing it.
// bundle is ever put in a prompt. Chat grounding intentionally uses only public-safe summaries and omits raw
// action rationale/blocker arrays, which can carry private readiness context. Mirrors github/commands.ts's
// publicBlockerDetail redaction intent without importing it.
const PRIVATE_DECISION_BLOCKER_PATTERN =
/\b(?:open_pr_pressure|closed_pr_credibility|low_credibility|maintainer_lane|inactive_or_unknown_lane|issue_discovery_only|merged_pr_history_floor|issue_discovery_validity_floor)\b/gi;
const PRIVATE_BOUNDARY_TERM_PATTERN =
Expand All @@ -67,8 +68,6 @@ type ChatGroundingAction = {
actionType: string;
status: string;
publicSafeSummary: string;
why: string[];
blockedBy: string[];
};

type ChatGroundingBundle = {
Expand Down Expand Up @@ -189,8 +188,6 @@ function compactChatSignalBundle(bundle: AgentRunBundle): ChatGroundingBundle {
actionType: action.actionType,
status: action.status,
publicSafeSummary: redactGroundingText(action.publicSafeSummary),
why: action.why.slice(0, 4).map(redactGroundingText).filter((line) => line.length > 0),
blockedBy: action.blockedBy.slice(0, 4).map(redactGroundingText).filter((line) => line.length > 0),
})),
freshnessWarnings: bundle.contextSnapshots.flatMap((snapshot) => snapshot.freshnessWarnings).slice(0, 8).map(redactGroundingText),
};
Expand Down
25 changes: 20 additions & 5 deletions test/unit/ai-chat-qa.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,8 @@ describe("generateChatQaAnswer", () => {
expect(userMessage).not.toMatch(/\bopen_pr_pressure\b/);
expect(userMessage).not.toMatch(/\bwallet\b/i);
expect(userMessage).not.toMatch(/\blikely_duplicate\b/);
expect(userMessage).not.toContain('"why"');
expect(userMessage).not.toContain('"blockedBy"');
});

it("honors a custom model override and clamps output tokens", async () => {
Expand Down Expand Up @@ -342,11 +344,24 @@ describe("__chatQaInternals", () => {
expect(redactGroundingText("perfectly safe text")).toBe("perfectly safe text");
});

it("compacts a bundle to at most 5 actions and filters out blank why/blockedBy lines after redaction", () => {
const compact = compactChatSignalBundle(bundleFixture());
expect(compact.actions).toHaveLength(1);
expect(compact.actions[0]?.why).toHaveLength(2);
expect(compact.actions[0]?.why.every((line) => line.length > 0)).toBe(true);
it("compacts a bundle using only public-safe action summaries", () => {
const compact = compactChatSignalBundle(
bundleFixture(undefined, {
why: ["Closed PR rate is 35%.", "direct PR lane share 0.37"],
blockedBy: ["closed_pr_credibility"],
publicSafeSummary: "Use the public preflight summary for contributor-visible context.",
}),
);
expect(compact.actions).toEqual([
{
actionType: "cleanup_existing_prs",
status: "recommended",
publicSafeSummary: "Use the public preflight summary for contributor-visible context.",
},
]);
expect(JSON.stringify(compact)).not.toContain("Closed PR rate");
expect(JSON.stringify(compact)).not.toContain("direct PR lane share");
expect(JSON.stringify(compact)).not.toContain("closed_pr_credibility");
expect(compact.freshnessWarnings).toEqual(["fresh enough"]);
});

Expand Down