close
Skip to content

fix: preserve UTF-8 in search_code source - #1253

Merged
DeusData merged 8 commits into
DeusData:mainfrom
anfedoro:agent/fix-search-code-utf8
Aug 17, 2026
Merged

fix: preserve UTF-8 in search_code source#1253
DeusData merged 8 commits into
DeusData:mainfrom
anfedoro:agent/fix-search-code-utf8

Conversation

@anfedoro

Copy link
Copy Markdown

Summary

  • Preserve valid UTF-8 in search_code full results and raw match content.
  • Replace the ASCII-only sanitizer that converted every byte >= 0x80 to ? with the existing UTF-8-safe lossy sanitizer.
  • Add a regression test using Russian Markdown content.

Root cause

search_code constructed result fields through sanitize_ascii(). Because UTF-8 characters are encoded as bytes >= 0x80, the sanitizer replaced their individual bytes with ? after the file and line match had already been found correctly.

Scope

This PR addresses result serialization after a successful match. It intentionally does not address the separate Windows CP_ACP pattern-matching issue discussed in #1209.

Related to #1209

Validation

  • make -f Makefile.cbm test-focused TEST_SUITES=mcp — 179 passed, 2 skipped.
  • Production build via scripts/build.sh on macOS arm64.
  • Manual search_code --mode full smoke test confirmed Cyrillic is preserved in source.

@DeusData DeusData added bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges ux/behavior Display bugs, docs, adoption UX labels Jul 24, 2026
@DeusData DeusData added this to the 0.9.1-rc milestone Jul 24, 2026
@DeusData DeusData added the priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. label Jul 24, 2026
@DeusData

Copy link
Copy Markdown
Owner

Thanks for opening the focused split requested in #1209. The exact patch stays in post-match serialization, reuses the existing sanitize_utf8_lossy implementation, and does not claim to fix Windows CP_ACP matching. No dependency, workflow, network, prompt, or raw-file-I/O surface is added. The Russian full-source fixture is a useful RED guard. Before exact-head approval, please add coverage for the other behavior claimed by the PR: raw match content, attached context, and one invalid UTF-8 sequence proving lossy replacement still yields valid JSON. Both commits currently lack visible DCO sign-offs, so the branch also needs a signed-off commit history and the complete CI result.

Signed-off-by: anfedoro <anfedoro@gmail.com>
@anfedoro
anfedoro force-pushed the agent/fix-search-code-utf8 branch from 57032ca to 05574ff Compare July 24, 2026 14:53
@anfedoro
anfedoro marked this pull request as ready for review July 29, 2026 07:17
@anfedoro
anfedoro requested a review from DeusData as a code owner July 29, 2026 07:17
@DeusData

Copy link
Copy Markdown
Owner

A quick note so this does not sit here looking like your problem: your red CI is not caused by your change — and a rebase will not fix it either.

The failure is tests/test_daemon_version.c:604 ASSERT(all_succeeded), a Windows concurrent conflict-log append test. It is a live flake on main itself, still failing as of yesterday — so it is ours to fix, not something you can clear by rebasing.

I have flagged it internally as a production-race candidate rather than a test to tune: under our own testing doctrine a test whose verdict depends on scheduler timing is a defect, and the right response is a deterministic repro plus a fix, not a wider budget.

So: nothing is needed from you on the CI front. Your PR is queued for a proper content review and I will come back to you on the change itself. Apologies that a failure of ours has been sitting on your PR looking like yours.

@DeusData
DeusData merged commit 9a547b3 into DeusData:main Aug 17, 2026
28 checks passed
@DeusData

Copy link
Copy Markdown
Owner

Merged — thank you! Swapping the four search_code sites onto the existing UTF-8-safe lossy sanitizer fixes real pain for every non-Latin codebase, and we took the context-column addition with it: a parameter that silently did nothing was its own small bug, and your tests cover both formats. The Windows CP_ACP matching gap you scoped out cleanly remains tracked in #1209.

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

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. ux/behavior Display bugs, docs, adoption UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants