From 7f62a38c7365e5479eb8174bc88851e1648492d3 Mon Sep 17 00:00:00 2001 From: "detail-app[bot]" <180357370+detail-app[bot]@users.noreply.github.com> Date: Fri, 25 Sep 2026 08:18:47 +0000 Subject: [PATCH 1/2] fix(openai): anchor PLACEHOLDER_RE so valid summaries mentioning "placeholder" are kept --- src/openai/requestSentence.test.ts | 151 +++++++++++++++++++++++++++++ src/openai/requestSentence.ts | 2 +- 2 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 src/openai/requestSentence.test.ts diff --git a/src/openai/requestSentence.test.ts b/src/openai/requestSentence.test.ts new file mode 100644 index 0000000..473d0df --- /dev/null +++ b/src/openai/requestSentence.test.ts @@ -0,0 +1,151 @@ +/** + * Offline regression tests for the `requestSentence` filter contract, focused on the + * PLACEHOLDER_RE bug where the first alternative was the unanchored bare word + * `placeholder`: substantive summaries that merely mentioned a placeholder (e.g. + * "Replaced the placeholder logo with a real project image") were discarded as if they + * were stand-in non-answers, silently dropping the row in both callers. + * + * No live DB or OpenAI access — the OpenAI client is stubbed with an object that + * returns a `submit_sentence` tool call carrying a crafted sentence, so the + * NON_ANSWER_RE -> PLACEHOLDER_RE pipeline is exercised through the public contract. + * + * Run with: + * npx ts-node src/openai/requestSentence.test.ts + * # or (no type-graphql decorator dependency, so transpile-only is safe): + * npx ts-node --transpile-only src/openai/requestSentence.test.ts + */ +import type OpenAIApi from 'openai'; +import { requestSentence } from './requestSentence'; +import type { OpenRouterChatCompletionCreateParams } from './requestSentence'; + +let failures = 0; + +function assert(condition: boolean, message: string): void { + if (!condition) { + failures += 1; + console.error(`FAILED: ${message}`); + } else { + console.log(`PASSED: ${message}`); + } +} + +function assertEqual(actual: T, expected: T, message: string): void { + const a = JSON.stringify(actual); + const e = JSON.stringify(expected); + if (a !== e) { + failures += 1; + console.error(`FAILED: ${message}\n expected: ${e}\n actual: ${a}`); + } else { + console.log(`PASSED: ${message}`); + } +} + +const PARAMS = { + model: 'test-model', + messages: [], +} as unknown as OpenRouterChatCompletionCreateParams; + +function makeCompletion(sentence: string): unknown { + return { + choices: [{ + message: { + tool_calls: [{ + type: 'function', + function: { + name: 'submit_sentence', + arguments: JSON.stringify({ sentence }), + }, + }], + }, + }], + }; +} + +async function requestWith(sentence: string): Promise { + const client = { chat: { completions: { create: async (): Promise => makeCompletion(sentence) } } }; + return requestSentence(client as unknown as OpenAIApi, PARAMS); +} + +// The reported bug: substantive summaries that mention "placeholder" mid-sentence must +// be returned verbatim, not misclassified as a stand-in and discarded. +async function testSubstantiveSentencesMentioningPlaceholderAreKept(): Promise { + const cases = [ + 'Replaced the placeholder logo with a real project image', + 'Swapped placeholder text on the homepage for real content', + 'Updated a placeholder license file with the correct MIT text', + 'Removed a stale placeholder constant that broke the build', + 'Filled in placeholder values for the new config file', + ]; + for (const c of cases) { + assertEqual(await requestWith(c), c, `substantive sentence mentioning placeholder is kept: "${c}"`); + } +} + +// Regression guard: leading stand-ins that start with "placeholder" (the genuine +// target of the first alternative) are still rejected after anchoring it. +async function testLeadingPlaceholderStandInsAreRejected(): Promise { + const cases = ['placeholder', 'Placeholder', 'Placeholder description', 'placeholder, not a real answer']; + for (const c of cases) { + assertEqual(await requestWith(c), null, `leading stand-in rejected: "${c}"`); + } +} + +// Regression guard: the other two PLACEHOLDER_RE branches ("no/not ... available/found" +// and "[I] could not/couldn't/was unable to ... find ...") still reject genuine +// non-answers (anchoring the first alternative must not perturb them). +async function testCouldNotFindAndNoInformationBranchesReject(): Promise { + const cases = [ + 'No information available', + "I couldn't find any details", + 'was unable to find the project', + ]; + for (const c of cases) { + assertEqual(await requestWith(c), null, `"couldn't find"/"no information" branch rejects: "${c}"`); + } +} + +// Sanity guard: NON_ANSWER_RE still rejects task-completion remarks. +async function testNonAnswerReRejects(): Promise { + const cases = ['I confirm the command has been executed', 'Task completed']; + for (const c of cases) { + assertEqual(await requestWith(c), null, `non-answer filter still rejects: "${c}"`); + } +} + +// Sanity guard: genuine substantive answers (research + PR-summary shapes) are +// returned verbatim. +async function testGenuineSubstantiveAnswersAreReturned(): Promise { + const cases = ["Used in Pixar's rendering pipeline.", 'Added input validation to the login form.']; + for (const c of cases) { + assertEqual(await requestWith(c), c, `genuine answer returned verbatim: "${c}"`); + } +} + +// Documents the narrowing tradeoff: a longer word starting with "placeholder" but with +// no word boundary right after (e.g. "placeholders") is NOT caught by `^placeholder\b` — +// the substance of such a sentence is kept. This is the intended consequence of using +// `\b` rather than re-anchoring on the bare substring. +async function testPluralPlaceholderPrefixedWordIsKeptByAnchor(): Promise { + assertEqual( + await requestWith('Removed obsolete placeholders from the config templates'), + 'Removed obsolete placeholders from the config templates', + 'anchored first alternative does not match a longer word starting with "placeholder"', + ); +} + +async function main(): Promise { + await testSubstantiveSentencesMentioningPlaceholderAreKept(); + await testLeadingPlaceholderStandInsAreRejected(); + await testCouldNotFindAndNoInformationBranchesReject(); + await testNonAnswerReRejects(); + await testGenuineSubstantiveAnswersAreReturned(); + await testPluralPlaceholderPrefixedWordIsKeptByAnchor(); + + if (failures > 0) { + console.error(`\n${failures} test(s) failed.`); + process.exit(1); + } + console.log('\nAll tests passed.'); +} + +main(); diff --git a/src/openai/requestSentence.ts b/src/openai/requestSentence.ts index 40241c5..3b0f1f7 100644 --- a/src/openai/requestSentence.ts +++ b/src/openai/requestSentence.ts @@ -49,7 +49,7 @@ const NON_ANSWER_RE = /^(i (confirm|have completed|have executed)|task (complete // still forced to call the tool, and some models satisfy that by submitting a stand-in // string instead of admitting they found nothing. Reject these the same way as a // non-answer, rather than saving them as if they were a real result. -const PLACEHOLDER_RE = /placeholder|^(no|not) (specific |concrete |publicly )?(information|data|details) (is |was |)?(available|found)|^(i )?(could not|couldn't|was unable to) find/i; +const PLACEHOLDER_RE = /^placeholder\b|^(no|not) (specific |concrete |publicly )?(information|data|details) (is |was |)?(available|found)|^(i )?(could not|couldn't|was unable to) find/i; /** * Requests a single short sentence from a chat model, forcing the answer through a From 646347fc1e004937c69ccd2b43fca5a318a2684c Mon Sep 17 00:00:00 2001 From: "detail-app[bot]" <180357370+detail-app[bot]@users.noreply.github.com> Date: Fri, 25 Sep 2026 08:26:23 +0000 Subject: [PATCH 2/2] fix(openai): keep substantive sentences that start with "placeholder" Narrow the leading-"placeholder" stand-in match to a bare token or one-word label so a real 5-10-word summary beginning with "placeholder" (e.g. "Placeholder logo was replaced with a real project image") is no longer dropped. Both callers mark the row fetched without retrying, so the dropped summary would be lost permanently. --- src/openai/requestSentence.test.ts | 20 ++++++++++++++++++-- src/openai/requestSentence.ts | 11 ++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/openai/requestSentence.test.ts b/src/openai/requestSentence.test.ts index 473d0df..baf3ef6 100644 --- a/src/openai/requestSentence.test.ts +++ b/src/openai/requestSentence.test.ts @@ -82,14 +82,29 @@ async function testSubstantiveSentencesMentioningPlaceholderAreKept(): Promise { - const cases = ['placeholder', 'Placeholder', 'Placeholder description', 'placeholder, not a real answer']; + const cases = ['placeholder', 'Placeholder', 'Placeholder description', 'placeholder text']; for (const c of cases) { assertEqual(await requestWith(c), null, `leading stand-in rejected: "${c}"`); } } +// The case the follow-up review raised: a substantive 5-10-word summary that starts with +// "placeholder" (as a regular word, e.g. describing a stand-in that was replaced) is a +// real answer, not a stand-in token, and must be kept — both callers mark the row +// fetched without retrying, so dropping it would lose it permanently. +async function testSubstantiveSentencesStartingWithPlaceholderAreKept(): Promise { + const cases = [ + 'Placeholder logo was replaced with a real project image', + 'Placeholder text swapped for finalized copy on the homepage', + ]; + for (const c of cases) { + assertEqual(await requestWith(c), c, `substantive sentence starting with placeholder is kept: "${c}"`); + } +} + // Regression guard: the other two PLACEHOLDER_RE branches ("no/not ... available/found" // and "[I] could not/couldn't/was unable to ... find ...") still reject genuine // non-answers (anchoring the first alternative must not perturb them). @@ -135,6 +150,7 @@ async function testPluralPlaceholderPrefixedWordIsKeptByAnchor(): Promise async function main(): Promise { await testSubstantiveSentencesMentioningPlaceholderAreKept(); + await testSubstantiveSentencesStartingWithPlaceholderAreKept(); await testLeadingPlaceholderStandInsAreRejected(); await testCouldNotFindAndNoInformationBranchesReject(); await testNonAnswerReRejects(); diff --git a/src/openai/requestSentence.ts b/src/openai/requestSentence.ts index 3b0f1f7..d184c33 100644 --- a/src/openai/requestSentence.ts +++ b/src/openai/requestSentence.ts @@ -49,7 +49,16 @@ const NON_ANSWER_RE = /^(i (confirm|have completed|have executed)|task (complete // still forced to call the tool, and some models satisfy that by submitting a stand-in // string instead of admitting they found nothing. Reject these the same way as a // non-answer, rather than saving them as if they were a real result. -const PLACEHOLDER_RE = /^placeholder\b|^(no|not) (specific |concrete |publicly )?(information|data|details) (is |was |)?(available|found)|^(i )?(could not|couldn't|was unable to) find/i; +// +// The `placeholder` stand-in is matched only as a bare token or a one-word label +// (`^placeholder\b(\s+\w+)?\W*$`): a substantive sentence that merely starts with the +// word "placeholder" (e.g. "Placeholder logo was replaced with a real project image") +// is a real summary, not a stand-in, and is kept. Matching the bare word alone would +// drop it, and both callers mark the row fetched without retrying, so the loss would be +// permanent. Explicit disclaimers longer than a bare label can't be told apart from a +// real sentence by shape alone, so they're let through rather than risk dropping a real +// summary. +const PLACEHOLDER_RE = /^placeholder\b(\s+\w+)?\W*$|^(no|not) (specific |concrete |publicly )?(information|data|details) (is |was |)?(available|found)|^(i )?(could not|couldn't|was unable to) find/i; /** * Requests a single short sentence from a chat model, forcing the answer through a