Skip to content

fix: land document-viewer review fixes missed by #492 squash-merge - #505

Merged
BigSimmo merged 4 commits into
mainfrom
claude/document-viewer-review-fixes
Jul 11, 2026
Merged

fix: land document-viewer review fixes missed by #492 squash-merge#505
BigSimmo merged 4 commits into
mainfrom
claude/document-viewer-review-fixes

Conversation

@BigSimmo

Copy link
Copy Markdown
Owner

Summary

Follow-up to #492. That PR was squash-merged while its head was still the reconciliation commit, so the four review fixes I pushed afterward (in response to CodeRabbit/Codex) did not make it into main. This PR lands exactly those fixes on top of current main (cherry-pick of the original review-fix commit; the three lib files are untouched on main since #492, so it applied cleanly). Each fix has a regression test.

  1. Negated contraindications → false danger badge (document-summary-badges.ts, was CodeRabbit Major + Codex P2): the Contraindications danger badge now runs through a hasPositiveContraindication guard that checks for a negation cue (no, not, non, without, nil, no known, …) before each contraindicat* match. "no contraindications" / "not contraindicated" no longer emit a red clinical-stop badge; a genuine contraindication still does. Clinical-safety relevant — an inverted danger badge is the highest-urgency signal in the summary cluster.

  2. Decimal doses classified as headings (indexed-source-formatting.ts, CodeRabbit Major): isNumberedHeading now requires a non-lowercase token after the numeric prefix (\s+(?![a-z])\S), so "12.5 mg" / "2.5 mmol/L" stay paragraphs while Title-Case and digit-led headings still resolve.

  3. CRLF handling (indexed-source-formatting.ts, CodeRabbit): flowIndexedText and parseIndexedSourceText normalize \r\n?\n atomically, so a Windows line ending is one newline, not a spurious blank line.

  4. Complete unpunctuated final sentence truncated (document-summary-formatting.ts, Codex P2): tail repair now only fires when the raw stored summary ended with a truncation ellipsis (captured before the sanitizer normalizes it away), so a complete final sentence lacking a period is left intact — no fabricated ellipsis, no false "trimmed" notice.

Verification

Clinical Governance Preflight

  • Source-backed claims still require linked source verification (display-only formatters; verbatim-quote path untouched)
  • No patient-identifiable workflow introduced
  • Supabase target unchanged (Clinical KB Database sjrfecxgysukkwxsowpy)
  • Service-role keys / private access remain server-only
  • Demo/synthetic content stays behind the fail-closed demo gate
  • Source metadata / conservative behavior preserved — this PR makes badge/heading/truncation behavior more conservative (fewer false danger signals, doses not promoted, no false truncation)
  • No clinical decision-support behavior change beyond the conservative fixes above

🤖 Generated with Claude Code

- 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 <noreply@anthropic.com>
@supabase

supabase Bot commented Jul 11, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ec4216d3-0899-42af-a720-39270865a29a

📥 Commits

Reviewing files that changed from the base of the PR and between 8b68c45 and e765b38.

📒 Files selected for processing (3)
  • src/lib/demo-data.ts
  • src/lib/document-summary-formatting.ts
  • tests/document-summary-formatting.test.ts
✅ Files skipped from review due to trivial changes (1)
  • src/lib/demo-data.ts

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved document summary badges to avoid danger flags for negated “contraindications” wording.
    • Enhanced summary formatting to better detect and repair truncated tails without adding unnecessary ellipses.
    • Corrected Windows CRLF handling to prevent unintended paragraph breaks.
    • Improved numbered-heading detection to avoid misclassifying decimal dose values and to better validate heading text.
  • Tests

    • Added/expanded regression coverage for badge negation handling, truncation repair behavior, numbered-heading detection, and CRLF line-break behavior.

Walkthrough

The changes refine document-summary badge matching, truncation-tail handling, numbered-heading detection, and newline normalization. Tests cover negated contraindications, truncated and complete endings, decimal values, and CRLF input.

Changes

Summary processing behavior

Layer / File(s) Summary
Negation-aware contraindication badges
src/lib/document-summary-badges.ts, tests/document-summary-badges.test.ts
Badge rules support optional guards, and negated contraindication phrases no longer produce danger badges.
Raw truncation-aware summary formatting
src/lib/document-summary-formatting.ts, src/lib/demo-data.ts, tests/document-summary-formatting.test.ts
Tail repair uses raw ellipsis and duplicate-prefix signals, with updated truncated demo data and regression coverage for complete and truncated endings.
Indexed heading and newline normalization
src/lib/indexed-source-formatting.ts, tests/indexed-source-formatting.test.ts
Lowercase-led numbered lines are excluded as headings, and CRLF/CR sequences are normalized as single line breaks.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 70.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (10 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: backporting review fixes missed by a squash-merge.
Description check ✅ Passed The description matches the template with Summary, Verification, and Clinical Governance sections and includes the required checklist items.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Generated And Sensitive Files ✅ Passed The PR only changes source/test TypeScript files; no secrets, .envs, caches, build outputs, node_modules, or other sensitive/generated artifacts appear in the diff.
Verification Claims ✅ Passed The PR names exact checks (tsc --noEmit, eslint, prettier --check, unit tests) and marks verify:ui as n/a with a reason.
Risky Git Or Deployment Actions ✅ Passed The PR only changes lib code and tests; no added instructions, scripts, workflows, or docs recommend force-push/reset/cleanup/deployment actions.
Supabase Project And Schema Safety ✅ Passed The PR only changes lib/test files for formatting/badge logic; it does not touch Supabase envs, migrations, schema, RLS, or project refs.
Runtime And Package Manager Integrity ✅ Passed PR diff touches only lib/tests; no package/runtime files changed. Repo still pins npm@11.17.0, Node 24, and engine-strict=true.
Api Route Failure Handling ✅ Passed Only lib formatting/demo-data and tests changed; no API routes, server actions, RAG/search, ingestion, or provider integrations were touched.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/document-viewer-review-fixes
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/document-viewer-review-fixes

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8b68c45a48

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/lib/document-summary-formatting.ts Outdated
coderabbitai[bot]
coderabbitai Bot previously requested changes Jul 11, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/document-summary-formatting.ts`:
- Around line 337-356: Update the tail-repair logic around orderedSections and
repairTruncatedCompactTail to normalize any existing terminal Unicode ellipsis
before appending the repair marker. Ensure the helper receives a single trailing
ellipsis rather than duplicated “…” markers, while preserving the existing
repaired-tail replacement and drop behavior.
🪄 Autofix (Beta)

✅ Autofix completed


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 87b4db36-bf70-4bb9-89fc-d7674ffffd30

📥 Commits

Reviewing files that changed from the base of the PR and between d14ef4b and 8b68c45.

📒 Files selected for processing (7)
  • src/lib/demo-data.ts
  • src/lib/document-summary-badges.ts
  • src/lib/document-summary-formatting.ts
  • src/lib/indexed-source-formatting.ts
  • tests/document-summary-badges.test.ts
  • tests/document-summary-formatting.test.ts
  • tests/indexed-source-formatting.test.ts

Comment thread src/lib/document-summary-formatting.ts Outdated
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Fixes Applied Successfully

Fixed 1 file(s) based on 1 unresolved review comment.

Files modified:

  • src/lib/document-summary-formatting.ts

Commit: d79e78ab738765c27c899be6f38a44d5a250f20c

The changes have been pushed to the claude/document-viewer-review-fixes branch.

Time taken: 2m 51s

coderabbitai Bot and others added 3 commits July 11, 2026 15:49
Fixed 1 file(s) based on 1 unresolved review comment.

Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
- 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 <noreply@anthropic.com>
…xes' into claude/document-viewer-review-fixes

# Conflicts:
#	src/lib/document-summary-formatting.ts
@BigSimmo

Copy link
Copy Markdown
Owner Author

Both follow-up findings on the tail-truncation logic are resolved in e765b38 (which also incorporates the CodeRabbit auto-fix commit):

  • Codex P2 — "keep repairing no-ellipsis truncated tails": right, the ellipsis-only gate over-corrected. The logic now repairs a truncated tail on either signal: (a) the raw stored summary ended with an ellipsis, or (b) the final sentence is a prefix of another kept sentence — a cut-off repeat like "Lithium is a narro" vs the full "…narrow therapeutic index drug." A complete, unique unpunctuated final sentence is still left intact (never mangled). New regression tests cover all three: no-ellipsis prefix-duplicate drop, explicit ellipsis repair, and complete-unpunctuated keep. The demo/test fixtures are back to the no-ellipsis shape.

  • CodeRabbit Minor — normalize terminal ellipsis before repair: the base now strips a terminal Unicode as well as ASCII dots (/[.\s…]+$/), so repairTruncatedCompactTail never receives a doubled … …. (This matches the auto-fix; it's included in the merged result.)

Verification: tsc / eslint / prettier clean; formatter, badges, indexed-source, and demo-data suites pass (41 focused + full suites green locally).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/document-summary-formatting.ts`:
- Around line 337-372: Update the tail-repair loop around rawEndedTruncated and
repairTruncatedCompactTail so rawEndedTruncated only enables repair when the
exact raw-derived truncated sentence remains as the final item after
keepNewSentences deduplication. Track that sentence before deduplication or
compare its identity/content with last, and otherwise rely on
isTruncatedDuplicate without rewriting the surviving valid sentence.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ec4216d3-0899-42af-a720-39270865a29a

📥 Commits

Reviewing files that changed from the base of the PR and between 8b68c45 and e765b38.

📒 Files selected for processing (3)
  • src/lib/demo-data.ts
  • src/lib/document-summary-formatting.ts
  • tests/document-summary-formatting.test.ts
✅ Files skipped from review due to trivial changes (1)
  • src/lib/demo-data.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/document-summary-formatting.ts`:
- Around line 337-372: Update the tail-repair loop around rawEndedTruncated and
repairTruncatedCompactTail so rawEndedTruncated only enables repair when the
exact raw-derived truncated sentence remains as the final item after
keepNewSentences deduplication. Track that sentence before deduplication or
compare its identity/content with last, and otherwise rely on
isTruncatedDuplicate without rewriting the surviving valid sentence.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ec4216d3-0899-42af-a720-39270865a29a

📥 Commits

Reviewing files that changed from the base of the PR and between 8b68c45 and e765b38.

📒 Files selected for processing (3)
  • src/lib/demo-data.ts
  • src/lib/document-summary-formatting.ts
  • tests/document-summary-formatting.test.ts
✅ Files skipped from review due to trivial changes (1)
  • src/lib/demo-data.ts
🛑 Comments failed to post (1)
src/lib/document-summary-formatting.ts (1)

337-372: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

sed -n '250,390p' src/lib/document-summary-formatting.ts

Repository: BigSimmo/Database

Length of output: 6321


🏁 Script executed:

rg -n "keepNewSentences|rawEndedTruncated|messyLithiumSummary|repairTruncatedCompactTail|normalizeSentenceKey" src test tests fixtures

Repository: BigSimmo/Database

Length of output: 3269


🏁 Script executed:

sed -n '145,220p' src/lib/document-summary-formatting.ts

Repository: BigSimmo/Database

Length of output: 3213


🏁 Script executed:

sed -n '500,575p' src/lib/source-text-sanitizer.ts

Repository: BigSimmo/Database

Length of output: 4661


🏁 Script executed:

sed -n '1,140p' tests/document-summary-formatting.test.ts

Repository: BigSimmo/Database

Length of output: 7443


🏁 Script executed:

rg -n "unsafeTruncationTailPattern" src/lib/source-text-sanitizer.ts src/lib/document-summary-formatting.ts

Repository: BigSimmo/Database

Length of output: 331


🏁 Script executed:

sed -n '1,120p' src/lib/source-text-sanitizer.ts

Repository: BigSimmo/Database

Length of output: 8542


Gate tail repair on the surviving raw tail sentence
rawEndedTruncated can still trigger repairTruncatedCompactTail after keepNewSentences drops the true truncated fragment as a long containment repeat, so the branch may rewrite the new last sentence and strip valid clinical text. Track the final raw-derived sentence before dedupe, or only repair when that exact item survives as the last item.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/document-summary-formatting.ts` around lines 337 - 372, Update the
tail-repair loop around rawEndedTruncated and repairTruncatedCompactTail so
rawEndedTruncated only enables repair when the exact raw-derived truncated
sentence remains as the final item after keepNewSentences deduplication. Track
that sentence before deduplication or compare its identity/content with last,
and otherwise rely on isTruncatedDuplicate without rewriting the surviving valid
sentence.

@BigSimmo
BigSimmo merged commit a518719 into main Jul 11, 2026
15 checks passed
@BigSimmo
BigSimmo deleted the claude/document-viewer-review-fixes branch July 13, 2026 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant