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
14 changes: 12 additions & 2 deletions review-enrichment/src/render-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,25 @@ export interface AnalyzerRenderHelpers {
bytesLabel(value: number | null): string;
}

// safeCodeSpan/promptText sit at the analyzer-output boundary: their signature promises `string`, but analyzer
// findings are loosely-typed data flowing through `as never` casts (registry.ts render()), not values this module
// controls -- a descriptor whose finding shape drifts (or a future config-driven table) can hand either helper a
// non-string. Coercing defensively here mirrors bytesLabel's own `value: number | null` guard below: never let a
// render helper throw into the brief pipeline (GITTENSORY-15, `value.replace is not a function`).
function asRenderableString(value: string): string {
return typeof value === "string" ? value : String(value ?? "");
}

export function safeCodeSpan(value: string): string {
return `\`${value.replace(
const safe = asRenderableString(value);
return `\`${safe.replace(
CODE_SPAN_UNSAFE,
(char) => CODE_SPAN_REPLACEMENTS[char] ?? "\ufffd",
)}\``;
}

export function promptText(value: string): string {
return value
return asRenderableString(value)
.replace(/[\u0000-\u001f\u007f]/g, " ")
.replace(/\\/g, "\\\\")
.replace(/`/g, "\\`")
Expand Down
58 changes: 58 additions & 0 deletions review-enrichment/test/render-helpers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Units for the shared analyzer-render helpers (render-helpers.ts). Own file (not render-contract.test.ts or
// terminology.test.ts) so it can cover the helpers directly as well as the boundary they sit behind. Regression
// coverage for GITTENSORY-15: "TypeError: value.replace is not a function" thrown from safeCodeSpan when a
// terminology finding's `term`/`suggestion` reached render as a non-string. safeCodeSpan/promptText are typed
// `(value: string): string`, but they receive values from less-trusted analyzer/descriptor output (routed through
// `as never` casts in render.ts/registry.ts), the same boundary bytesLabel already defends with its
// `value: number | null` guard -- so both now defensively coerce instead of trusting the type signature.
import { test } from "node:test";
import assert from "node:assert/strict";

import { safeCodeSpan, promptText } from "../dist/render-helpers.js";
import { renderBrief } from "../dist/render.js";

test("safeCodeSpan: wraps a normal string in backticks unchanged", () => {
assert.equal(safeCodeSpan("src/net.ts:12"), "`src/net.ts:12`");
});

test("safeCodeSpan: escapes backticks and control characters in a real string", () => {
assert.equal(safeCodeSpan("a`b\nc"), "`aˋb␤c`");
});

test("safeCodeSpan: coerces a non-string value instead of throwing (GITTENSORY-15 regression)", () => {
assert.equal(safeCodeSpan(undefined), "``");
assert.equal(safeCodeSpan(null), "``");
assert.equal(safeCodeSpan(42), "`42`");
assert.equal(safeCodeSpan(["master", "slave"]), "`master,slave`");
assert.equal(safeCodeSpan({ toString: () => "obj" }), "`obj`");
});

test("promptText: escapes markdown-sensitive characters in a real string", () => {
assert.equal(promptText("a*b_c"), "a\\*b\\_c");
});

test("promptText: coerces a non-string value instead of throwing (GITTENSORY-15 regression)", () => {
assert.equal(promptText(undefined), "");
assert.equal(promptText(null), "");
assert.equal(promptText(7), "7");
});

test("renderBrief: terminology descriptor render survives a non-string term/suggestion without throwing", () => {
// Mirrors the exact GITTENSORY-15 stack trace: registry.ts's "terminology" descriptor template calls
// safeCodeSpan(item.term) and safeCodeSpan(item.suggestion) directly (no template-literal coercion), unlike
// the `${item.file}:${item.line}` interpolation just before it. Simulates a malformed finding (e.g. from a
// future config-driven term table or an upstream payload drift) reaching the render path unchanged.
assert.doesNotThrow(() => {
// Cast through `any` (not `@ts-expect-error`) since test/**/*.ts is outside tsconfig.json's "src"-only
// include and is run via --experimental-strip-types (no type-check on test files) -- an unused
// `@ts-expect-error` here would be inert, not a compile guard.
const malformedFinding = { file: "src/net.ts", line: 1, term: undefined, suggestion: 42 };
const { promptSection } = renderBrief({
terminology: [malformedFinding],
});
assert.match(promptSection, /Non-inclusive terminology/);
assert.match(promptSection, /src\/net\.ts:1/);
assert.match(promptSection, /``/); // the coerced-empty term renders as an empty code span
assert.match(promptSection, /`42`/); // the coerced-number suggestion renders as `42`
});
});
Loading