Skip to content

Fix #5537: Fix capCodePoints returning nearly the full string when maxCodePoints is zero or negative - #5538

Open
pamod-madubashana wants to merge 3 commits into
apache:mainfrom
pamod-madubashana:pamod-madubashana/issue-5537-35505003700
Open

pamod-madubashana wants to merge 3 commits into
apache:mainfrom
pamod-madubashana:pamod-madubashana/issue-5537-35505003700

Conversation

@pamod-madubashana

Copy link
Copy Markdown

Fixes #5537

PR Summary — Fix capCodePoints for zero/negative caps (#5537)

What changed

  • packages/core/src/thread-search.tscapCodePoints now handles maxCodePoints <= 0 explicitly:
    if (maxCodePoints <= 0) return codePoints.length === 0 ? value : '…';
    inserted after the existing no-truncation short-circuit and before the slice(0, maxCodePoints - 1) truncation. Non-empty input with a zero or negative cap now returns just "…", and empty input still returns "" (previously "" with a negative cap fell through to the slice and incorrectly returned "…").

Why it addresses the issue

  • The old truncation codePoints.slice(0, maxCodePoints - 1) used -1 as the end index for a 0 cap, and a negative end counts back from the array end — so capCodePoints("hello", 0) returned "hell…" (5 code points) instead of "…", completely bypassing the documented "at most maxCodePoints code points" bound. Negative caps behaved the same way.
  • The new guard returns before the slice whenever the cap is non-positive, so the slice end can never go negative on that path, matching the 0-cap behavior of the sibling helper sanitizeUnicodeText. Positive-cap behavior is untouched (still slice(0, max - 1) + '…'), and current production callers passing SNIPPET_MAX_CODE_POINTS = 240 are unaffected.

Verification

  • The repository's test suite could not be run in this environment: dependencies are not installed (node_modules absent), so the @maka/core build fails with error TS2688: Cannot find type definition file for 'node', and the thread-search tests (which run from built output via node --test dist/main/**/*.test.js) cannot execute.
  • Instead, the fixed capCodePoints function was extracted verbatim from packages/core/src/thread-search.ts and evaluated directly with node -e, covering the issue's repro plus edge cases:
    • "hello" @0 => "…" (1pt) (was "hell…" (5pts) before the fix)
    • "0123456789…"(40 chars) @0 => "…" (1pt) (was 40pts before the fix)
    • "hello" @-2 => "…" (1pt) (was "he…" (3pts) before the fix)
    • "" @0 => "", "" @-2 => "" (empty input stays empty)
    • "hello" @1 => "…", "hello" @5 => "hello", "hello" @10 => "hello" (positive/no-truncation paths unchanged)
  • Result: all cases pass; zero/negative caps are now bounded as documented.

@github-actions github-actions Bot added the effort/XS Under 10 readable lines label Sep 20, 2026
@pamod-madubashana
pamod-madubashana marked this pull request as ready for review September 20, 2026 15:40
@ggbdpq

ggbdpq commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Two small things, one of them semantic.

The <= 0 branch should return '', not '…'. The function's contract is "at most maxCodePoints code points", and '…' is itself one code point — so capCodePoints('hello', 0) returning '…' still exceeds a zero cap and becomes indistinguishable from the (correct) maxCodePoints = 1 result. The sibling helpers cited in the issue already set the precedent: truncateUtf16Safe returns '' for maxUnits <= 0, and sanitizeUnicodeText returns just the suffix (i.e. '') for a 0 cap. A single if (maxCodePoints <= 0) return ''; is simpler, satisfies the contract for every input including '', and makes the empty-input ternary unnecessary.

fork-error.txt / fork-view-error.txt look like accidental commits — both are empty files unrelated to the fix; worth dropping from the branch.

It would also be great to pin the boundary with a regression test (cap 0, -2, and '' input), since missing boundary coverage is how #5537 shipped.

…files, add regression tests

- capCodePoints now returns '' when maxCodePoints <= 0, matching the
  contract 'at most maxCodePoints code points' and sibling helpers
  truncateUtf16Safe/sanitizeUnicodeText (apache#5537)
- Remove empty fork-error.txt and fork-view-error.txt (accidental commits)
- Add regression tests for cap 0, -2, and empty string input
@pamod-madubashana

Copy link
Copy Markdown
Author

Thanks for the review, @ggbdpq! Here's what I addressed in the follow-up commit (a2fa940):

  1. Semantic fixcapCodePoints now returns '' (not '…') when maxCodePoints <= 0. This satisfies the "at most maxCodePoints code points" contract: capCodePoints('hello', 0) yields 0 code points, not 1. Matches the precedent in truncateUtf16Safe and sanitizeUnicodeText.

  2. Dropped accidental filesfork-error.txt and fork-view-error.txt removed from the branch.

  3. Regression test — Pinned boundary behavior for cap 0, -2, and '' input so Fix capCodePoints returning nearly the full string when maxCodePoints is zero or negative #5537 can't ship again.

@ggbdpq

ggbdpq commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Confirmed on a2fa940 — the '' return matches the contract and the sibling helpers, the stray files are gone, and the boundary regression covers cap 0 / -2 / empty input. Looks good, thanks for the quick turnaround.

@pamod-madubashana

pamod-madubashana commented Sep 21, 2026

Copy link
Copy Markdown
Author

Resolved merge conflict with upstream/main — the two modified files (packages/core/src/thread-search.ts and apps/desktop/src/main/__tests__/thread-search.test.ts) were deleted upstream in #5531 (feat/search). Conflict resolved by re-adding both files with the fix intact:

  • capCodePoints returns '' (not '…') when maxCodePoints <= 0
  • Regression test covers cap 0, -2, and empty string input

@github-actions github-actions Bot added effort/XL Under 2500 readable lines and removed effort/XS Under 10 readable lines labels Sep 22, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Under 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix capCodePoints returning nearly the full string when maxCodePoints is zero or negative

2 participants