From 8b68c45a48ac9f190dbb762d876412261e6de38d Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Sat, 11 Jul 2026 22:47:40 +0800 Subject: [PATCH 1/3] fix: address CodeRabbit/Codex review findings on PR #492 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - badges: contraindication danger badge is now negation-aware — "no contraindications" / "not contraindicated" no longer emit a false red clinical stop signal (hasPositiveContraindication guard). - indexed-source: isNumberedHeading no longer classifies decimal dose/value lines ("12.5 mg", "2.5 mmol/L") as section headings (require a non-lowercase token after the numeric prefix). - indexed-source: flowIndexedText + parseIndexedSourceText normalize CRLF/CR atomically (\r\n? -> \n) so Windows line endings don't become blank lines. - summary formatter: only repair/flag a truncated tail when the RAW stored summary actually ended with an ellipsis; a complete final sentence lacking punctuation is left intact (no fabricated ellipsis, no false "trimmed" notice). Demo fixture updated to a realistic "…narro..." truncation. - Regression tests added for all four. Co-Authored-By: Claude Fable 5 --- src/lib/demo-data.ts | 2 +- src/lib/document-summary-badges.ts | 27 ++++++++++++++- src/lib/document-summary-formatting.ts | 41 ++++++++++++++--------- src/lib/indexed-source-formatting.ts | 33 +++++++++++------- tests/document-summary-badges.test.ts | 17 ++++++++++ tests/document-summary-formatting.test.ts | 14 +++++++- tests/indexed-source-formatting.test.ts | 22 ++++++++++++ 7 files changed, 125 insertions(+), 31 deletions(-) diff --git a/src/lib/demo-data.ts b/src/lib/demo-data.ts index 8f929b6fb2..1f0c7cbd3b 100644 --- a/src/lib/demo-data.ts +++ b/src/lib/demo-data.ts @@ -393,7 +393,7 @@ export const demoDocumentSummaries: DocumentSummary[] = [ "levels every 3 months, renal and thyroid tests every 6 months, and calcium annually. The therapeutic " + "effect occurs gradually and may take up to three weeks. Escalate review for vomiting, diarrhoea, " + "dehydration, acute kidney injury, new NSAID/ACE inhibitor/diuretic exposure, tremor, confusion, or " + - "ataxia. therapeutic effect occurs gradually and may take up to three weeks. Lithium is a narro", + "ataxia. therapeutic effect occurs gradually and may take up to three weeks. Lithium is a narro...", clinical_specifics: {}, source_chunk_ids: [], source_image_ids: [], diff --git a/src/lib/document-summary-badges.ts b/src/lib/document-summary-badges.ts index d77dc8abe7..3ac059d42d 100644 --- a/src/lib/document-summary-badges.ts +++ b/src/lib/document-summary-badges.ts @@ -50,13 +50,37 @@ type SummaryPhraseRule = { label: string; tone: SemanticTone; iconKey?: SemanticIconKey; + // Optional second gate applied after `pattern` matches. Used where a raw + // keyword match would be clinically misleading (e.g. negated contraindications). + guard?: (text: string) => boolean; }; +// A negation cue close before a "contraindicat*" mention ("no contraindications", +// "not contraindicated in pregnancy") inverts its meaning — it must never emit a +// red danger badge, which would read as a false clinical stop signal. +const contraindicationNegationBefore = /\b(?:no|not|non|without|nil|free of|absence of|no known)\b[\s\w,'’-]{0,16}$/i; + +function hasPositiveContraindication(text: string): boolean { + const pattern = /contraindicat/gi; + let match: RegExpExecArray | null; + while ((match = pattern.exec(text)) !== null) { + const before = text.slice(Math.max(0, match.index - 40), match.index); + if (!contraindicationNegationBefore.test(before)) return true; + } + return false; +} + // Phrase catalogue over the stored summary text. Every rule here is also // registered in SEMANTIC_FLAG_CATALOGUE (document domain) so the // /reference/colour-coding legend stays complete. const summaryPhraseRules: SummaryPhraseRule[] = [ - { id: "summary-contraindication", pattern: /contraindicat/i, label: "Contraindications", tone: "danger" }, + { + id: "summary-contraindication", + pattern: /contraindicat/i, + label: "Contraindications", + tone: "danger", + guard: hasPositiveContraindication, + }, { id: "summary-narrow-therapeutic-index", pattern: /narrow therapeutic (?:index|window|range)/i, @@ -126,6 +150,7 @@ export function buildDocumentSummaryBadges({ if (summaryText) { for (const rule of summaryPhraseRules) { if (!rule.pattern.test(summaryText)) continue; + if (rule.guard && !rule.guard(summaryText)) continue; push({ id: rule.id, label: rule.label, tone: rule.tone, iconKey: rule.iconKey }); } } diff --git a/src/lib/document-summary-formatting.ts b/src/lib/document-summary-formatting.ts index 6685374f98..34ff6b0519 100644 --- a/src/lib/document-summary-formatting.ts +++ b/src/lib/document-summary-formatting.ts @@ -271,6 +271,13 @@ function splitIntoRawSections(text: string): RawSection[] { export function formatDocumentSummary(raw: string | null | undefined): FormattedDocumentSummary { if (!raw || !raw.trim()) return EMPTY_SUMMARY; + // The stored-summary truncation signal is a trailing ellipsis on the RAW text + // (the pre-fix retrieval_synopsis cut, e.g. "where poss..."). The sanitizer + // below normalizes that ellipsis into ". ", so capture it first — a complete + // final sentence that merely lacks punctuation has no raw ellipsis and must + // never be treated as truncated. + const rawEndedTruncated = /(?:\.{3}|…)\s*$/.test(raw.trim()); + // Reuse the house sanitizer first (glyph repair, protective markings, source // codes, label noise), then flatten to a single line for sentence work. const cleaned = cleanClinicalSummaryText(raw).replace(/\s+/g, " ").trim(); @@ -327,22 +334,26 @@ export function formatDocumentSummary(raw: string | null | undefined): Formatted if (headingKey) seenHeadings.set(headingKey, section); } - // Repair or drop a mid-word truncated tail on the very last item. - for (let index = orderedSections.length - 1; index >= 0; index -= 1) { - const items = orderedSections[index].items; - if (!items.length) continue; - const last = items[items.length - 1]; - const endsWithEllipsis = /(?:\.{3}|…)\s*$/.test(last); - const endsCleanly = /[.!?:;)\]"']$/.test(last.trim()); - if (endsCleanly && !endsWithEllipsis) break; - const repaired = repairTruncatedCompactTail(endsWithEllipsis ? last : `${last} ...`); - truncatedTail = true; - if (repaired && repaired.split(/\s+/).length >= 5) { - items[items.length - 1] = repaired; - } else { - items.pop(); + // Repair or drop a tail that was cut mid-thought at indexing. Only acted on + // when the RAW stored summary actually ended with a truncation ellipsis, so a + // complete final sentence lacking punctuation is never dropped or mis-flagged. + if (rawEndedTruncated) { + for (let index = orderedSections.length - 1; index >= 0; index -= 1) { + const items = orderedSections[index].items; + if (!items.length) continue; + const last = items[items.length - 1]; + // The sanitizer already turned the raw "…" into a plain period; restore an + // ellipsis so repairTruncatedCompactTail can drop the partial final token. + const base = last.replace(/[.\s]+$/, ""); + const repaired = repairTruncatedCompactTail(`${base} ...`); + truncatedTail = true; + if (repaired && repaired.split(/\s+/).length >= 5) { + items[items.length - 1] = repaired; + } else { + items.pop(); + } + break; } - break; } const sections = orderedSections.filter((section) => section.items.length > 0); diff --git a/src/lib/indexed-source-formatting.ts b/src/lib/indexed-source-formatting.ts index e49e54c92b..d3680bdc92 100644 --- a/src/lib/indexed-source-formatting.ts +++ b/src/lib/indexed-source-formatting.ts @@ -15,8 +15,10 @@ function isPageFooter(line: string) { function isNumberedHeading(line: string) { // Supports multi-level numbering ("2.7. Dosage", "3.1.2 Monitoring") as well // as top-level "9. Polypharmacy". A bare number ("12 hours") is not a - // heading — top-level numbering must carry its dot. - return /^\d{1,2}(?:(?:\.\d{1,2})+\.?|\.)\s+\S/.test(line.trim()) && line.trim().length <= 96; + // heading — top-level numbering must carry its dot. The (?![a-z]) guard + // rejects decimal measurements ("12.5 mg", "2.5 mmol/L") whose unit token is + // lowercase, while still accepting Title-Case and digit-led heading text. + return /^\d{1,2}(?:(?:\.\d{1,2})+\.?|\.)\s+(?![a-z])\S/.test(line.trim()) && line.trim().length <= 96; } function isLikelyTitle(rawLine: string, line: string, index: number) { @@ -191,21 +193,26 @@ export function mergeContinuationBlocks(blocks: IndexedTextBlock[]): IndexedText // paragraph breaks unless they sit mid-sentence (next line starts lowercase), // which is how extraction separates soft-wrapped continuations. export function flowIndexedText(text: string): string { - return text - .replace(/\r/g, "\n") - .replace(/[ \t]+\n[ \t]*/g, "\n") - .replace(/\n+/g, (run: string, offset: number, full: string) => { - if (run.length === 1) return " "; - const next = full.charAt(offset + run.length); - return /[a-z(]/.test(next) ? " " : "\n\n"; - }) - .replace(/[ \t]{2,}/g, " ") - .trim(); + return ( + text + // Normalize CRLF/CR atomically so a Windows line ending becomes ONE newline, + // not two (a lone `\r → \n` would turn `\r\n` into a spurious blank line). + .replace(/\r\n?/g, "\n") + .replace(/[ \t]+\n[ \t]*/g, "\n") + .replace(/\n+/g, (run: string, offset: number, full: string) => { + if (run.length === 1) return " "; + const next = full.charAt(offset + run.length); + return /[a-z(]/.test(next) ? " " : "\n\n"; + }) + .replace(/[ \t]{2,}/g, " ") + .trim() + ); } export function parseIndexedSourceText(text: string): IndexedTextBlock[] { const rawLines = text - .replace(/\r/g, "\n") + // CRLF/CR → single newline atomically (avoids spurious blank lines from `\r\n`). + .replace(/\r\n?/g, "\n") .split("\n") .map((line) => line.replace(/\s+$/g, "")); const blocks: IndexedTextBlock[] = []; diff --git a/tests/document-summary-badges.test.ts b/tests/document-summary-badges.test.ts index 5f7e1bf425..b9243a7364 100644 --- a/tests/document-summary-badges.test.ts +++ b/tests/document-summary-badges.test.ts @@ -66,6 +66,23 @@ describe("buildDocumentSummaryBadges", () => { expect(badges[0]).toBe(contraindication); }); + it("does not emit a danger badge for negated contraindication text", () => { + // A red Contraindications badge on negated text is a false clinical stop signal. + for (const summaryText of [ + "There are no contraindications to this therapy.", + "Lithium is not contraindicated in mild renal impairment.", + "No known contraindications have been reported.", + ]) { + const badges = buildDocumentSummaryBadges({ summaryText }); + expect(badges.find((badge) => badge.label === "Contraindications")).toBeUndefined(); + } + // Still fires when a genuine (non-negated) contraindication is described. + const positive = buildDocumentSummaryBadges({ + summaryText: "No dose adjustment needed, but it is contraindicated in severe hepatic failure.", + }); + expect(positive.find((badge) => badge.label === "Contraindications")?.tone).toBe("danger"); + }); + it("deduplicates equivalent label- and phrase-derived badges by display label", () => { const badges = buildDocumentSummaryBadges({ labels: [label({ label: "high-risk medication", label_type: "risk", confidence: 0.9 })], diff --git a/tests/document-summary-formatting.test.ts b/tests/document-summary-formatting.test.ts index 26d514693f..e33baf59c0 100644 --- a/tests/document-summary-formatting.test.ts +++ b/tests/document-summary-formatting.test.ts @@ -20,7 +20,7 @@ const messyLithiumSummary = "The therapeutic effect occurs gradually and may take up to three weeks. Lithium is a narrow therapeutic " + "index drug. It is handled by the body in a similar way to sodium; most risk factors for toxicity relate to " + "changes in sodium levels and fluid status. therapeutic effect occurs gradually and may take up to three " + - "weeks. Lithium is a narro"; + "weeks. Lithium is a narro..."; describe("stripSummaryBoilerplate", () => { it("removes the glued document-header run while keeping the first clinical sentence", () => { @@ -137,4 +137,16 @@ describe("formatDocumentSummary", () => { ); expect(formatted.sections.every((section) => section.heading === null)).toBe(true); }); + + it("keeps a complete final sentence that merely lacks terminal punctuation", () => { + // Regression: an unpunctuated but complete final sentence must not be + // fabricated into an ellipsis, truncated, or flagged as trimmed. + const formatted = formatDocumentSummary( + "This guideline covers clozapine initiation. Weekly monitoring continues for 18 weeks", + ); + const allText = [formatted.lead, ...formatted.sections.flatMap((s) => s.items)].filter(Boolean).join(" "); + expect(allText).toContain("Weekly monitoring continues for 18 weeks"); + expect(allText).not.toMatch(/…$/); + expect(formatted.truncatedTail).toBe(false); + }); }); diff --git a/tests/indexed-source-formatting.test.ts b/tests/indexed-source-formatting.test.ts index 0ba80740e0..68d043a36c 100644 --- a/tests/indexed-source-formatting.test.ts +++ b/tests/indexed-source-formatting.test.ts @@ -119,6 +119,20 @@ factors i.e. weight, comorbidities (e.g. renal impairment) and concomitant medic ); }); + it("does not classify decimal dose/value lines as numbered headings", () => { + for (const dose of ["12.5 mg daily", "2.5 mmol/L threshold", "0.5 mg at night"]) { + expect(parseIndexedSourceText(dose)).toContainEqual(expect.objectContaining({ type: "paragraph", text: dose })); + expect(parseIndexedSourceText(dose).some((block) => block.type === "heading")).toBe(false); + } + // Genuine numbered headings (Title-Case or digit-led) still resolve. + expect(parseIndexedSourceText("9. Polypharmacy")).toContainEqual( + expect.objectContaining({ type: "heading", text: "9. Polypharmacy" }), + ); + expect(parseIndexedSourceText("2.7. Dosage (as lithium carbonate)")).toContainEqual( + expect.objectContaining({ type: "heading", text: "2.7. Dosage (as lithium carbonate)" }), + ); + }); + it("merges unterminated paragraphs but never merges across headings or tables", () => { const heading: IndexedTextBlock = { type: "heading", id: "h", text: "1. Scope", level: "section" }; const merged = mergeContinuationBlocks([ @@ -161,4 +175,12 @@ describe("flowIndexedText", () => { "can reduce lithium clearance and therefore increase lithium levels.", ); }); + + it("treats a Windows CRLF as a single line break, not a blank line", () => { + // \r\n must collapse to one newline (→ one space when flowing), not a + // paragraph break, so CRLF-sourced excerpts read identically to LF ones. + expect(flowIndexedText("Escalate review when there is vomiting,\r\ndiarrhoea, dehydration, or ataxia.")).toBe( + "Escalate review when there is vomiting, diarrhoea, dehydration, or ataxia.", + ); + }); }); From d79e78ab738765c27c899be6f38a44d5a250f20c Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Sat, 11 Jul 2026 15:49:06 +0000 Subject: [PATCH 2/3] fix: apply CodeRabbit auto-fixes Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit --- src/lib/document-summary-formatting.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/document-summary-formatting.ts b/src/lib/document-summary-formatting.ts index 34ff6b0519..575a5d709a 100644 --- a/src/lib/document-summary-formatting.ts +++ b/src/lib/document-summary-formatting.ts @@ -344,7 +344,8 @@ export function formatDocumentSummary(raw: string | null | undefined): Formatted const last = items[items.length - 1]; // The sanitizer already turned the raw "…" into a plain period; restore an // ellipsis so repairTruncatedCompactTail can drop the partial final token. - const base = last.replace(/[.\s]+$/, ""); + // Strip any trailing periods, Unicode ellipsis characters, and whitespace. + const base = last.replace(/[.\s…]+$/, ""); const repaired = repairTruncatedCompactTail(`${base} ...`); truncatedTail = true; if (repaired && repaired.split(/\s+/).length >= 5) { From 3c2408a246566a9ba38681e99ae6dcae1dd95cde Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Sat, 11 Jul 2026 23:51:22 +0800 Subject: [PATCH 3/3] fix: refine summary tail-truncation per #505 review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Handle no-ellipsis truncated tails again (Codex): a final fragment that is a prefix of another kept sentence ("Lithium is a narro" vs the full "…narrow therapeutic index drug") is a cut-off repeat — drop + flag it, while still leaving a complete, unique unpunctuated sentence intact. - Strip a terminal Unicode "…" as well as ASCII dots before re-marking, so repairTruncatedCompactTail never receives a doubled "… …" (CodeRabbit). - Tests: no-ellipsis prefix-duplicate drop, explicit ellipsis repair, and the complete-unpunctuated-sentence keep case; demo fixture reverted to a no-ellipsis truncated repeat. Co-Authored-By: Claude Fable 5 --- src/lib/demo-data.ts | 2 +- src/lib/document-summary-formatting.ts | 54 +++++++++++++++-------- tests/document-summary-formatting.test.ts | 26 ++++++++++- 3 files changed, 60 insertions(+), 22 deletions(-) diff --git a/src/lib/demo-data.ts b/src/lib/demo-data.ts index 1f0c7cbd3b..90ec52845d 100644 --- a/src/lib/demo-data.ts +++ b/src/lib/demo-data.ts @@ -393,7 +393,7 @@ export const demoDocumentSummaries: DocumentSummary[] = [ "levels every 3 months, renal and thyroid tests every 6 months, and calcium annually. The therapeutic " + "effect occurs gradually and may take up to three weeks. Escalate review for vomiting, diarrhoea, " + "dehydration, acute kidney injury, new NSAID/ACE inhibitor/diuretic exposure, tremor, confusion, or " + - "ataxia. therapeutic effect occurs gradually and may take up to three weeks. Lithium is a narro...", + "ataxia. therapeutic effect occurs gradually and may take up to three weeks. Careful patient selection and monitori", clinical_specifics: {}, source_chunk_ids: [], source_image_ids: [], diff --git a/src/lib/document-summary-formatting.ts b/src/lib/document-summary-formatting.ts index 34ff6b0519..a390a68af8 100644 --- a/src/lib/document-summary-formatting.ts +++ b/src/lib/document-summary-formatting.ts @@ -334,26 +334,42 @@ export function formatDocumentSummary(raw: string | null | undefined): Formatted if (headingKey) seenHeadings.set(headingKey, section); } - // Repair or drop a tail that was cut mid-thought at indexing. Only acted on - // when the RAW stored summary actually ended with a truncation ellipsis, so a - // complete final sentence lacking punctuation is never dropped or mis-flagged. - if (rawEndedTruncated) { - for (let index = orderedSections.length - 1; index >= 0; index -= 1) { - const items = orderedSections[index].items; - if (!items.length) continue; - const last = items[items.length - 1]; - // The sanitizer already turned the raw "…" into a plain period; restore an - // ellipsis so repairTruncatedCompactTail can drop the partial final token. - const base = last.replace(/[.\s]+$/, ""); - const repaired = repairTruncatedCompactTail(`${base} ...`); - truncatedTail = true; - if (repaired && repaired.split(/\s+/).length >= 5) { - items[items.length - 1] = repaired; - } else { - items.pop(); - } - break; + // Repair or drop a tail cut mid-thought at indexing. Two truncation signals, + // both conservative so a complete, unique final sentence that merely lacks + // punctuation is never dropped or mis-flagged: + // (a) the RAW stored summary ended with an ellipsis marker (captured above, + // before the sanitizer normalized it into a plain period); or + // (b) the final sentence is a prefix of another kept sentence — a cut-off + // repeat, e.g. "Lithium is a narro" vs "Lithium is a narrow … drug". + for (let index = orderedSections.length - 1; index >= 0; index -= 1) { + const items = orderedSections[index].items; + if (!items.length) continue; + const last = items[items.length - 1]; + const endsCleanly = /[.!?:;)\]"']$/.test(last.trim()); + const lastKey = normalizeSentenceKey(last); + const isTruncatedDuplicate = + !endsCleanly && + lastKey.length >= 8 && + orderedSections.some((section) => + section.items.some((other) => { + if (other === last) return false; + const otherKey = normalizeSentenceKey(other); + return otherKey.length > lastKey.length && otherKey.startsWith(lastKey); + }), + ); + if (!rawEndedTruncated && !isTruncatedDuplicate) break; + // Strip any terminal dots/ellipsis the sanitizer left (ASCII "..." or a + // Unicode "…"), then re-mark so repairTruncatedCompactTail drops the partial + // final token exactly once (avoids a doubled "… …"). + const base = last.replace(/[.\s…]+$/, ""); + const repaired = repairTruncatedCompactTail(`${base} ...`); + truncatedTail = true; + if (repaired && repaired.split(/\s+/).length >= 5) { + items[items.length - 1] = repaired; + } else { + items.pop(); } + break; } const sections = orderedSections.filter((section) => section.items.length > 0); diff --git a/tests/document-summary-formatting.test.ts b/tests/document-summary-formatting.test.ts index e33baf59c0..b8862496c8 100644 --- a/tests/document-summary-formatting.test.ts +++ b/tests/document-summary-formatting.test.ts @@ -20,7 +20,7 @@ const messyLithiumSummary = "The therapeutic effect occurs gradually and may take up to three weeks. Lithium is a narrow therapeutic " + "index drug. It is handled by the body in a similar way to sodium; most risk factors for toxicity relate to " + "changes in sodium levels and fluid status. therapeutic effect occurs gradually and may take up to three " + - "weeks. Lithium is a narro..."; + "weeks. Lithium is a narro"; describe("stripSummaryBoilerplate", () => { it("removes the glued document-header run while keeping the first clinical sentence", () => { @@ -139,7 +139,7 @@ describe("formatDocumentSummary", () => { }); it("keeps a complete final sentence that merely lacks terminal punctuation", () => { - // Regression: an unpunctuated but complete final sentence must not be + // Regression: an unpunctuated but complete, unique final sentence must not be // fabricated into an ellipsis, truncated, or flagged as trimmed. const formatted = formatDocumentSummary( "This guideline covers clozapine initiation. Weekly monitoring continues for 18 weeks", @@ -149,4 +149,26 @@ describe("formatDocumentSummary", () => { expect(allText).not.toMatch(/…$/); expect(formatted.truncatedTail).toBe(false); }); + + it("drops a no-ellipsis final fragment that is a cut-off repeat of a full sentence", () => { + // "…is a narro" is a prefix of the earlier full "…is a narrow therapeutic + // index drug." sentence — a truncated repeat that must be removed and flagged, + // even without a trailing ellipsis. + const formatted = formatDocumentSummary( + "Lithium is a narrow therapeutic index drug. Monitor serum levels every three months. Lithium is a narro", + ); + const allText = [formatted.lead, ...formatted.sections.flatMap((s) => s.items)].filter(Boolean).join(" "); + expect(allText).toContain("narrow therapeutic index drug"); + expect(allText).not.toMatch(/is a narro$/); + expect(formatted.truncatedTail).toBe(true); + }); + + it("repairs an explicit trailing-ellipsis truncation", () => { + const formatted = formatDocumentSummary( + "Baseline renal and thyroid function must be checked. Doses are titrated to serum lithium levels which should be measured where poss...", + ); + const allText = [formatted.lead, ...formatted.sections.flatMap((s) => s.items)].filter(Boolean).join(" "); + expect(allText).not.toMatch(/where poss/); + expect(formatted.truncatedTail).toBe(true); + }); });