Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Gittensory CI and gittensory review score, gate, and comment on pull requests. T

- **`Gittensory Orb Review Agent`** (`gate.*` / `settings.gateCheckMode` / `settings.reviewCheckMode`, off by default) — the authoritative GitHub Check Run carrying the gate's pass/fail verdict. This is the one worth making a required status check.
- **`Gittensory Context`** (`settings.checkRunMode` / `settings.checkRunDetailLevel`, off by default) — a separate, purely advisory Check Run. At its default `checkRunDetailLevel: minimal` it publishes no findings at all; even at `standard`/`deep` it only re-renders content already shown elsewhere. Never make this one required.
- **Inline review comments** (`GITTENSORY_REVIEW_INLINE_COMMENTS` + `.gittensory.yml`'s `review.inline_comments`, off by both by default) — real, reply-able line-anchored PR review comment threads (CodeRabbit-style). This is the ONLY one of the three that posts an interactive per-line thread; the two check runs above never do. With `.gittensory.yml`'s `review.suggestions` also on, a precise line-anchored fix is additionally rendered as a one-click, committable GitHub suggested-change block.
- **Inline review comments** (`GITTENSORY_REVIEW_INLINE_COMMENTS` + `.gittensory.yml`'s `review.inline_comments`, off by both by default) — real, reply-able line-anchored PR review comment threads (CodeRabbit-style). This is the ONLY one of the three that posts an interactive per-line thread; the two check runs above never do. With `.gittensory.yml`'s `review.suggestions` also on, a precise line-anchored fix is additionally rendered as a one-click, committable GitHub suggested-change block. With `review.finding_categories` also on (off by default), each finding is additionally tagged with a category — security/correctness/performance/maintainability/tests/style — in both the inline comment label and the unified comment's "Finding categories" collapsible; a deterministic path/keyword fallback covers whatever the model omits.

See [Tuning your reviews](https://gittensory.aethereal.dev/docs/tuning) for the full flag, setting, and `.gittensory.yml` reference.

Expand Down
32 changes: 27 additions & 5 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ import {
} from "../services/ai-review";
import {
maybePostInlineComments,
shouldRenderFindingCategories,
shouldRenderSuggestions,
shouldRequestInlineFindings,
} from "../review/inline-comments";
Expand Down Expand Up @@ -6426,6 +6427,10 @@ export async function runAiReviewForAdvisory(
// (the per-repo toggle). ANDed here with the operator flag + cutover allowlist to decide whether to ASK the
// model for line-anchored inline findings. Absent/false ⇒ the reviewer prompt is byte-identical (no findings).
reviewInlineComments?: boolean | undefined;
// `.gittensory.yml` review.finding_categories (#1958), resolved by the caller from the cached manifest. ANDed
// here with reviewInlineComments (a category has nothing to categorize without an inline finding) to decide
// whether to ASK the model to self-categorize each inlineFindings item. Absent/false ⇒ byte-identical prompt.
reviewFindingCategories?: boolean | undefined;
// `.gittensory.yml` review.ai_model (#selfhost-ai-model-override), resolved by the caller from the cached
// manifest. Self-host only — overrides that repo's claude-code/codex model+effort, taking priority over the
// operator's global env vars. Absent/all-null ⇒ byte-identical (global env var, then provider default).
Expand Down Expand Up @@ -6654,6 +6659,13 @@ export async function runAiReviewForAdvisory(
diff: enrichmentDiff,
})
: undefined;
// Resolved once and reused for BOTH inlineFindings itself and the finding-categories opt-in layered on top
// of it (#1958) — a category has nothing to categorize without an inline finding to attach it to.
const inlineFindingsRequested = shouldRequestInlineFindings(
env,
args.repoFullName,
args.reviewInlineComments,
);
const result = await runGittensoryAiReview(env, {
repoFullName: args.repoFullName,
prNumber: args.pr.number,
Expand Down Expand Up @@ -6686,11 +6698,10 @@ export async function runAiReviewForAdvisory(
codexEffort: args.reviewSelfHostAiModel?.codexEffort ?? null,
// Inline comments (#inline-comments): ask the model for line-anchored findings only when the operator flag,
// the cutover allowlist, AND the per-repo manifest toggle all pass. Otherwise the prompt is byte-identical.
inlineFindings: shouldRequestInlineFindings(
env,
args.repoFullName,
args.reviewInlineComments,
),
inlineFindings: inlineFindingsRequested,
// review.finding_categories (#1958): ask the model to ALSO self-categorize each inlineFindings item, only
// when inline findings themselves are being requested (a category has nothing to categorize otherwise).
findingCategories: shouldRenderFindingCategories(inlineFindingsRequested, args.reviewFindingCategories),
pathGuidance: resolveReviewPathInstructions(
args.reviewPathInstructions ?? [],
files.map((file) => file.path),
Expand Down Expand Up @@ -7640,6 +7651,7 @@ async function maybePublishPrPublicSurface(
let inlineCommentsEnabledForReview = false;
let suggestionsEnabledForReview = false;
let changedFilesSummaryEnabledForReview = false;
let findingCategoriesEnabledForReview = false;
let aiReviewExpected = false;
let aiReviewWasReused = false;
let gateFinalized = false;
Expand Down Expand Up @@ -8247,6 +8259,7 @@ async function maybePublishPrPublicSurface(
securityFocus: reviewSecurityFocus,
inlineComments: reviewInlineComments,
suggestions: reviewSuggestions,
findingCategories: reviewFindingCategories,
pathInstructions: reviewPathInstructions,
instructions: manifestReviewInstructions,
tone: reviewTone,
Expand All @@ -8263,6 +8276,10 @@ async function maybePublishPrPublicSurface(
inlineCommentsEnabledForReview,
reviewSuggestions,
);
findingCategoriesEnabledForReview = shouldRenderFindingCategories(
inlineCommentsEnabledForReview,
reviewFindingCategories,
);
const reviewFilesForAi = await getReviewFiles();
const changedPaths = reviewFilesForAi.map((file) => file.path);
// Per-repo review CONTEXT (#review-skills): fold the container-private review/AGENTS.md (or legacy
Expand Down Expand Up @@ -8453,6 +8470,7 @@ async function maybePublishPrPublicSurface(
reviewExcludePaths,
reviewPathFilters,
reviewInlineComments,
reviewFindingCategories,
reviewSelfHostAiModel,
deliveryId: webhook.deliveryId,
});
Expand Down Expand Up @@ -9311,6 +9329,9 @@ async function maybePublishPrPublicSurface(
})),
}
: {}),
...(findingCategoriesEnabledForReview && aiReview?.inlineFindings?.length
? { findingCategories: aiReview.inlineFindings }
: {}),
});
} else {
deterministicBody = buildPublicPrIntelligenceComment(commentArgs);
Expand Down Expand Up @@ -9366,6 +9387,7 @@ async function maybePublishPrPublicSurface(
mode,
inlineCommentsEnabled: inlineCommentsEnabledForReview,
suggestionsEnabled: suggestionsEnabledForReview,
categoriesEnabled: findingCategoriesEnabledForReview,
});
}
if (decision.willLabel) {
Expand Down
42 changes: 42 additions & 0 deletions src/review/finding-category-classify.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { isTestFile } from "../signals/local-branch";
import { isTestPath } from "../signals/test-evidence";

// Deterministic category taxonomy for AI review findings (#1958). The model is asked to self-categorize each
// inlineFinding when review.finding_categories is on; `classifyFindingCategory` supplies the SAFE DEFAULT for
// whatever it omits or mis-emits, so a caller with the feature on always has a category to render — never a
// sometimes-present field. Pure, path/keyword-only — no diff content, no IO.

export const FINDING_CATEGORIES = ["security", "correctness", "performance", "maintainability", "tests", "style"] as const;

export type FindingCategory = (typeof FINDING_CATEGORIES)[number];

/** Type guard for a model-provided `category` value — anything outside the fixed enum (wrong case, a made-up
* category, a non-string) is rejected so the caller falls back to {@link classifyFindingCategory}. */
export function isFindingCategory(value: unknown): value is FindingCategory {
return typeof value === "string" && (FINDING_CATEGORIES as readonly string[]).includes(value);
}

const SECURITY_KEYWORDS =
/\b(?:sql injection|xss|cross-site scripting|csrf|authentication|authorization|secret|credential|vulnerab\w*|sanitiz\w*|command injection|path traversal|ssrf|deserializ\w*|hardcoded (?:password|key|token)|insecure)\b/i;
const PERFORMANCE_KEYWORDS =
/\b(?:performance|\bslow\b|n\+1|memory leak|inefficient|redundant (?:call|fetch|query)|unnecessary re-?render|blocking call|latency|throughput)\b/i;
const TEST_KEYWORDS = /\b(?:test coverage|missing test|flaky test|test case|assertion)\b/i;
const STYLE_KEYWORDS = /\b(?:naming|formatting|whitespace|indentation|lint\w*|style guide|typo)\b/i;
const MAINTAINABILITY_KEYWORDS =
/\b(?:duplicat\w*|refactor\w*|readability|overly complex|magic number|dead code|unused (?:variable|import|function))\b/i;

/**
* Deterministic fallback categorization (#1958): PATH first (a finding anchored to a test file is a "tests"
* finding regardless of wording), then keyword sniffing over the finding's own body text, ordered so the
* costliest miscategorization (missing a real security defect) is checked first. Falls through to
* "correctness" — the general "this is a bug" bucket — when nothing else matches. Pure.
*/
export function classifyFindingCategory(finding: { path: string; body: string }): FindingCategory {
if (isTestPath(finding.path) || isTestFile(finding.path)) return "tests";
if (SECURITY_KEYWORDS.test(finding.body)) return "security";
if (PERFORMANCE_KEYWORDS.test(finding.body)) return "performance";
if (TEST_KEYWORDS.test(finding.body)) return "tests";
if (STYLE_KEYWORDS.test(finding.body)) return "style";
if (MAINTAINABILITY_KEYWORDS.test(finding.body)) return "maintainability";
return "correctness";
}
39 changes: 29 additions & 10 deletions src/review/inline-comments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import { createPullRequestReviewComments } from "../github/pr-actions";
import { isConvergenceRepoAllowed } from "./cutover-gate";
import { classifyFindingCategory } from "./finding-category-classify";
import type { InlineFinding } from "../services/ai-review";
import type { AgentActionMode } from "../settings/agent-execution";
import type { PullRequestFileRecord } from "../types";
Expand Down Expand Up @@ -45,6 +46,16 @@ export function shouldRenderSuggestions(
return inlineCommentsEnabled && manifestToggle === true;
}

/** PURE (#1958): should an inline finding's `category` be rendered? An ADDITIONAL opt-in (`review.finding_categories`)
* layered on top of inline comments being enabled at all — mirrors {@link shouldRenderSuggestions} exactly, since
* a category has nothing to categorize without the inline comment it rides on. */
export function shouldRenderFindingCategories(
inlineCommentsEnabled: boolean,
manifestToggle: boolean | undefined,
): boolean {
return inlineCommentsEnabled && manifestToggle === true;
}

/** A GitHub inline review comment anchored to a line on the RIGHT (added/context) side of the PR diff. */
export type ReviewInlineComment = { path: string; line: number; side: "RIGHT"; body: string };

Expand Down Expand Up @@ -88,23 +99,28 @@ function safeSuggestionBlock(suggestion: string | undefined): string {
return `\n\n\`\`\`suggestion\n${suggestion}\n\`\`\``;
}

/** The inline comment body: a compact severity label + the finding, plus a one-click GitHub suggested-change
* block when the finding carries a `suggestion` AND the caller has suggestions enabled (#1956). Public-safe by
* construction — both the body and the suggestion were already run through the public-safe filter by
* composeInlineFindings before they reached here. */
function formatInlineBody(finding: InlineFinding, suggestionsEnabled: boolean): string {
/** The inline comment body: a compact severity (+ optional category) label + the finding, plus a one-click GitHub
* suggested-change block when the finding carries a `suggestion` AND the caller has suggestions enabled (#1956).
* When `categoriesEnabled` (#1958), the label gets a parenthetical category tag — the model's own `category` when
* it emitted one in the fixed enum, else the deterministic fallback (`classifyFindingCategory`), so the tag is
* never sometimes-present. Public-safe by construction — both the body and the suggestion were already run
* through the public-safe filter by composeInlineFindings before they reached here; `category` is a fixed enum
* literal, never free text. */
function formatInlineBody(finding: InlineFinding, suggestionsEnabled: boolean, categoriesEnabled = false): string {
const label = finding.severity === "blocker" ? "Blocker" : "Nit";
const categoryTag = categoriesEnabled ? ` (${finding.category ?? classifyFindingCategory(finding)})` : "";
const suggestionBlock = suggestionsEnabled ? safeSuggestionBlock(finding.suggestion) : "";
return `**${label}:** ${finding.body}${suggestionBlock}`;
return `**${label}${categoryTag}:** ${finding.body}${suggestionBlock}`;
}

/** PURE: turn the model's line-anchored findings into GitHub inline review comments, dropping any whose
* (path, line) is not a commentable RIGHT-side line in that file's diff (so GitHub never 422s) and any file with
* no usable patch. Dedupes by path+line (first wins) and caps the total. Empty in / nothing anchorable ⇒ [].
* `suggestionsEnabled` (#1956) gates whether a finding's `suggestion` is rendered as a committable GitHub
* suggested-change block — a suggestion is anchored to the SAME single line as its parent finding, so the
* existing line-validity check above already covers "drop it if the range can't be anchored". */
export function selectInlineComments(findings: InlineFinding[], files: Pick<PullRequestFileRecord, "path" | "payload">[], suggestionsEnabled = false): ReviewInlineComment[] {
* existing line-validity check above already covers "drop it if the range can't be anchored". `categoriesEnabled`
* (#1958) gates whether the label carries a category tag. */
export function selectInlineComments(findings: InlineFinding[], files: Pick<PullRequestFileRecord, "path" | "payload">[], suggestionsEnabled = false, categoriesEnabled = false): ReviewInlineComment[] {
const rightLinesByPath = new Map<string, Set<number>>();
for (const file of files) {
const patch = typeof file.payload?.patch === "string" ? file.payload.patch : "";
Expand All @@ -119,7 +135,7 @@ export function selectInlineComments(findings: InlineFinding[], files: Pick<Pull
const key = `${finding.path}:${finding.line}`;
if (seen.has(key)) continue;
seen.add(key);
out.push({ path: finding.path, line: finding.line, side: "RIGHT", body: formatInlineBody(finding, suggestionsEnabled) });
out.push({ path: finding.path, line: finding.line, side: "RIGHT", body: formatInlineBody(finding, suggestionsEnabled, categoriesEnabled) });
}
return out;
}
Expand All @@ -139,9 +155,10 @@ export async function postInlineReviewComments(
files: Pick<PullRequestFileRecord, "path" | "payload">[];
mode: AgentActionMode;
suggestionsEnabled?: boolean | undefined;
categoriesEnabled?: boolean | undefined;
},
): Promise<{ posted: number }> {
const comments = selectInlineComments(args.findings, args.files, args.suggestionsEnabled);
const comments = selectInlineComments(args.findings, args.files, args.suggestionsEnabled, args.categoriesEnabled);
if (comments.length === 0 || !args.commitId) return { posted: 0 };
try {
await createPullRequestReviewComments(env, args.installationId, args.repoFullName, args.pullNumber, args.commitId, comments, args.mode);
Expand Down Expand Up @@ -170,6 +187,7 @@ export async function maybePostInlineComments(
mode: AgentActionMode;
inlineCommentsEnabled: boolean;
suggestionsEnabled?: boolean | undefined;
categoriesEnabled?: boolean | undefined;
},
): Promise<void> {
if (!args.inlineCommentsEnabled) return;
Expand All @@ -184,5 +202,6 @@ export async function maybePostInlineComments(
files: await args.getFiles(),
mode: args.mode,
suggestionsEnabled: args.suggestionsEnabled,
categoriesEnabled: args.categoriesEnabled,
});
}
Loading
Loading