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.
- **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.

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

Expand Down
8 changes: 8 additions & 0 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,
shouldRenderSuggestions,
shouldRequestInlineFindings,
} from "../review/inline-comments";
import { evaluateClaCheck } from "../review/cla-check";
Expand Down Expand Up @@ -7637,6 +7638,7 @@ async function maybePublishPrPublicSurface(
}
| undefined;
let inlineCommentsEnabledForReview = false;
let suggestionsEnabledForReview = false;
let aiReviewExpected = false;
let aiReviewWasReused = false;
let gateFinalized = false;
Expand Down Expand Up @@ -8236,6 +8238,7 @@ async function maybePublishPrPublicSurface(
profile: reviewProfile,
securityFocus: reviewSecurityFocus,
inlineComments: reviewInlineComments,
suggestions: reviewSuggestions,
pathInstructions: reviewPathInstructions,
instructions: manifestReviewInstructions,
tone: reviewTone,
Expand All @@ -8248,6 +8251,10 @@ async function maybePublishPrPublicSurface(
repoFullName,
reviewInlineComments,
);
suggestionsEnabledForReview = shouldRenderSuggestions(
inlineCommentsEnabledForReview,
reviewSuggestions,
);
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 @@ -9341,6 +9348,7 @@ async function maybePublishPrPublicSurface(
getFiles: getReviewFiles,
mode,
inlineCommentsEnabled: inlineCommentsEnabledForReview,
suggestionsEnabled: suggestionsEnabledForReview,
});
}
if (decision.willLabel) {
Expand Down
47 changes: 39 additions & 8 deletions src/review/inline-comments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@ export function shouldRequestInlineFindings(
return manifestToggle === true && isInlineCommentsEnabled(env) && isConvergenceRepoAllowed(env, repoFullName);
}

/** PURE (#1956): should a `suggestion` be rendered as a GitHub-native ` ```suggestion ` block? This is an
* ADDITIONAL opt-in (`review.suggestions`) layered on top of inline comments being enabled at all — a
* suggestion has nothing to attach to without the inline comment it rides on, so it can never be true when
* `inlineCommentsEnabled` is false, regardless of the manifest toggle. */
export function shouldRenderSuggestions(
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 @@ -66,17 +77,34 @@ export function rightSideLinesFromPatch(patch: string): Set<number> {
return lines;
}

/** The inline comment body: a compact severity label + the finding. Public-safe by construction — the body was
* already run through the public-safe filter by composeInlineFindings before it reached here. */
function formatInlineBody(finding: InlineFinding): string {
/** GitHub's suggested-change syntax requires the LITERAL ` ```suggestion ` fence; if the suggestion text itself
* contains a triple-backtick run, embedding it verbatim would prematurely close the fence and corrupt the
* comment (the rest of the finding body would spill out as raw, unintended markdown). Fail-safe (#1956):
* drop the suggestion block and keep the finding text rather than risk a malformed comment — mirrors the
* "a bad/blank suggestion is simply dropped while keeping the finding itself" discipline already applied when
* the suggestion is parsed (ai-review.ts's parseModelReview). */
function safeSuggestionBlock(suggestion: string | undefined): string {
if (!suggestion || suggestion.includes("```")) return "";
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 {
const label = finding.severity === "blocker" ? "Blocker" : "Nit";
return `**${label}:** ${finding.body}`;
const suggestionBlock = suggestionsEnabled ? safeSuggestionBlock(finding.suggestion) : "";
return `**${label}:** ${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 ⇒ []. */
export function selectInlineComments(findings: InlineFinding[], files: Pick<PullRequestFileRecord, "path" | "payload">[]): ReviewInlineComment[] {
* 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[] {
const rightLinesByPath = new Map<string, Set<number>>();
for (const file of files) {
const patch = typeof file.payload?.patch === "string" ? file.payload.patch : "";
Expand All @@ -91,7 +119,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) });
out.push({ path: finding.path, line: finding.line, side: "RIGHT", body: formatInlineBody(finding, suggestionsEnabled) });
}
return out;
}
Expand All @@ -110,9 +138,10 @@ export async function postInlineReviewComments(
findings: InlineFinding[];
files: Pick<PullRequestFileRecord, "path" | "payload">[];
mode: AgentActionMode;
suggestionsEnabled?: boolean | undefined;
},
): Promise<{ posted: number }> {
const comments = selectInlineComments(args.findings, args.files);
const comments = selectInlineComments(args.findings, args.files, args.suggestionsEnabled);
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 @@ -140,6 +169,7 @@ export async function maybePostInlineComments(
getFiles: () => Promise<Pick<PullRequestFileRecord, "path" | "payload">[]>;
mode: AgentActionMode;
inlineCommentsEnabled: boolean;
suggestionsEnabled?: boolean | undefined;
},
): Promise<void> {
if (!args.inlineCommentsEnabled) return;
Expand All @@ -153,5 +183,6 @@ export async function maybePostInlineComments(
findings,
files: await args.getFiles(),
mode: args.mode,
suggestionsEnabled: args.suggestionsEnabled,
});
}
24 changes: 18 additions & 6 deletions src/signals/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,12 @@ export type FocusManifestReviewConfig = {
* comments = byte-identical behavior. Operator-gated too (GITTENSORY_REVIEW_INLINE_COMMENTS + allowlist).
* (#inline-comments) */
inlineComments: boolean | null;
/** `review.suggestions`: when true, an inline finding whose AI-provided fix is precise enough to anchor to a
* single line is ALSO rendered as a GitHub-native ` ```suggestion ` block a contributor can commit in one
* click. Only takes effect when inline comments are already on (a suggestion has nothing to attach to
* otherwise) — this is an ADDITIONAL opt-in on top of `review.inline_comments`, not a replacement gate.
* null/false (default, absent) = no suggestion blocks = byte-identical behavior. (#1956) */
suggestions: boolean | null;
/** `review.path_instructions`: per-path natural-language guidance handed to the AI reviewer when the PR's
* changed files match the glob. Empty (default) ⇒ byte-identical reviewer prompt. (#review-path-instructions) */
pathInstructions: ReviewPathInstruction[];
Expand Down Expand Up @@ -568,7 +574,7 @@ const EMPTY_MANIFEST: FocusManifest = {
publicNotes: [],
gate: { ...EMPTY_GATE_CONFIG },
settings: {},
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } },
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } },
features: { ...EMPTY_FEATURES_CONFIG },
contentLane: { ...EMPTY_CONTENT_LANE_CONFIG },
repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG },
Expand Down Expand Up @@ -598,7 +604,7 @@ function emptyManifest(source: FocusManifestSource, warnings: string[] = []): Fo
warnings,
gate: { ...EMPTY_GATE_CONFIG },
settings: {},
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } },
review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } },
features: { ...EMPTY_FEATURES_CONFIG },
contentLane: { ...EMPTY_CONTENT_LANE_CONFIG },
repoDocGeneration: { ...EMPTY_REPO_DOC_GENERATION_CONFIG },
Expand Down Expand Up @@ -1528,7 +1534,7 @@ function parsePublicSafeText(value: JsonValue | undefined, field: string, warnin
* throws; invalid/unsafe values are dropped with warnings.
*/
function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): FocusManifestReviewConfig {
const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } };
const empty: FocusManifestReviewConfig = { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, suggestions: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, labelingRules: [], aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG } };
if (value === undefined || value === null) return empty;
if (typeof value !== "object" || Array.isArray(value)) {
warnings.push(`Manifest field "review" must be a mapping; ignoring it.`);
Expand Down Expand Up @@ -1565,6 +1571,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
const tone = parsePublicSafeText(r.tone, "review.tone", warnings);
const securityFocus = normalizeOptionalBoolean(r.security_focus, "review.security_focus", warnings);
const inlineComments = normalizeOptionalBoolean(r.inline_comments, "review.inline_comments", warnings);
const suggestions = normalizeOptionalBoolean(r.suggestions, "review.suggestions", warnings);
const pathInstructions = parseReviewPathInstructions(r.path_instructions, warnings);
const instructions = parsePublicSafeText(r.instructions, "review.instructions", warnings);
const excludePaths = parseReviewExcludePaths(r.exclude_paths, warnings);
Expand All @@ -1581,6 +1588,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
tone !== null ||
securityFocus !== null ||
inlineComments !== null ||
suggestions !== null ||
pathInstructions.length > 0 ||
instructions !== null ||
excludePaths.length > 0 ||
Expand All @@ -1601,6 +1609,7 @@ function parseReviewConfig(value: JsonValue | undefined, warnings: string[]): Fo
tone,
securityFocus,
inlineComments,
suggestions,
pathInstructions,
instructions,
excludePaths,
Expand Down Expand Up @@ -1918,6 +1927,7 @@ export function reviewConfigToJson(review: FocusManifestReviewConfig): JsonValue
if (review.tone !== null) out.tone = review.tone;
if (review.securityFocus !== null) out.security_focus = review.securityFocus;
if (review.inlineComments !== null) out.inline_comments = review.inlineComments;
if (review.suggestions !== null) out.suggestions = review.suggestions;
if (review.instructions !== null) out.instructions = review.instructions;
if (review.pathInstructions.length > 0) out.path_instructions = review.pathInstructions.map((entry) => ({ path: entry.path, instructions: entry.instructions }));
if (review.excludePaths.length > 0) out.exclude_paths = [...review.excludePaths];
Expand Down Expand Up @@ -2055,12 +2065,14 @@ export function composeManifestReviewInstructions(instructions: string | null, t
* `review.exclude_paths` + `review.path_filters` + `review.ai_model`) from a possibly-null manifest (null = load
* failure). A null manifest yields the byte-identical defaults. Centralized so the AI-review caller threads them
* in one place with the null-manifest branch covered here (unit-tested) rather than inline in the processor.
* (#review-profile / #review-tone / #review-security-focus / #review-path-instructions / #review-exclude-paths / #2043 / #selfhost-ai-model-override) */
export function resolveReviewPromptOverrides(manifest: FocusManifest | null): { profile: ReviewProfile | null; tone: string | null; securityFocus: boolean; inlineComments: boolean; pathInstructions: ReviewPathInstruction[]; instructions: string | null; excludePaths: string[]; pathFilters: string[]; selfHostAiModel: SelfHostAiModelConfig } {
* (#review-profile / #review-tone / #review-security-focus / #review-path-instructions / #review-exclude-paths / #2043 / #selfhost-ai-model-override / #1956) */
export function resolveReviewPromptOverrides(manifest: FocusManifest | null): { profile: ReviewProfile | null; tone: string | null; securityFocus: boolean; inlineComments: boolean; suggestions: boolean; pathInstructions: ReviewPathInstruction[]; instructions: string | null; excludePaths: string[]; pathFilters: string[]; selfHostAiModel: SelfHostAiModelConfig } {
// inlineComments resolves to a strict boolean — true ONLY when the manifest explicitly set review.inline_comments:
// true; null/false/absent ⇒ false. The caller ANDs this per-repo toggle with the operator flag + cutover allowlist.
// securityFocus resolves the same way — true ONLY when the manifest explicitly set review.security_focus: true.
return { profile: manifest?.review.profile ?? null, tone: manifest?.review.tone ?? null, securityFocus: manifest?.review.securityFocus === true, inlineComments: manifest?.review.inlineComments === true, pathInstructions: manifest?.review.pathInstructions ?? [], instructions: manifest?.review.instructions ?? null, excludePaths: manifest?.review.excludePaths ?? [], pathFilters: manifest?.review.pathFilters ?? [], selfHostAiModel: resolveReviewSelfHostAiModel(manifest) };
// suggestions resolves the same way (#1956) — the caller further ANDs it with the already-resolved
// inlineComments gate, since a suggestion has nothing to attach to without an inline comment.
return { profile: manifest?.review.profile ?? null, tone: manifest?.review.tone ?? null, securityFocus: manifest?.review.securityFocus === true, inlineComments: manifest?.review.inlineComments === true, suggestions: manifest?.review.suggestions === true, pathInstructions: manifest?.review.pathInstructions ?? [], instructions: manifest?.review.instructions ?? null, excludePaths: manifest?.review.excludePaths ?? [], pathFilters: manifest?.review.pathFilters ?? [], selfHostAiModel: resolveReviewSelfHostAiModel(manifest) };
}

/** Resolve `review.pre_merge_checks` from a possibly-null manifest (null = load failure ⇒ no checks). Centralized
Expand Down
Loading
Loading