[timoncool]

LM output goes through byte-level BPE, so bpe_decode returns raw bytes. When generation stops in the middle of a multi-byte character (Cyrillic, CJK, an emoji at the token limit), the caption or lyrics end in an incomplete UTF-8 sequence. request_to_json then fails inside yyjson (invalid UTF-8 is refused by default), gets NULL with a length of 0 and returns an empty string, so /lm answers [] with status done: a client sees a finished job that planned nothing, and nothing in the log says why.

• utf8_valid (src/bpe.h): an incomplete sequence at the end is dropped, any other ill-formed byte becomes U+FFFD (overlongs, surrogates and code points above U+10FFFF included). bpe_decode returns its text through it, so the LM phase-1 text and the understand text are always valid UTF-8. Both callers decode the whole token sequence at once, so no character is cut between calls.
• request_to_json and request_write check the yyjson result and log the reason; request_to_json returns an empty string on failure, as its header now says.
• /lm and /understand fail the job with a FATAL line when a result cannot be written, instead of answering [].
• tests/test-utf8.cpp covers the repaired and the untouched cases.

Summary by CodeRabbit

• Bug Fixes
  • Decoded text now handles malformed UTF-8 more safely: invalid sequences are replaced with the replacement character, and incomplete trailing sequences are omitted.
  • Requests that cannot be serialized are reported as failures instead of producing incomplete results.
• Tests
  • Added checks for valid text across several writing systems and for malformed or incomplete UTF-8 sequences.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: fb886194-ba9f-4b0b-86ca-7195ebf3111f

📥 Commits

Reviewing files that changed from the base of the PR and between de858d8 and b0b2f21.

📒 Files selected for processing: 7

• src/bpe.h
• src/request.cpp
• src/request.h
• src/sampling.h
• tests/CMakeLists.txt
• tests/test-utf8.cpp
• tools/ace-server.cpp

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The change adds UTF-8 validation for decoded BPE text, reports JSON serialization failures, and updates LM and understand workers to fail jobs when result serialization returns an empty string. It also adds tests for valid, malformed, and incomplete UTF-8 sequences.

Changes: UTF-8-safe result serialization

Layer / File(s)	Summary
Validate decoded UTF-8 src/bpe.h, src/sampling.h, tests/test-utf8.cpp, tests/CMakeLists.txt	utf8_valid preserves valid UTF-8, replaces malformed bytes with U+FFFD, and drops incomplete trailing sequences. bpe_decode uses the validator. Tests cover these cases and valid text.
Report request serialization failures src/request.cpp, src/request.h	request_write logs serialization errors, closes the file, and returns false on failure. request_to_json logs errors and returns an empty string.
Fail jobs on result serialization errors tools/ace-server.cpp	The LM and understand workers mark jobs FAILED and return when result serialization returns an empty string.

Priority: ➖ Normal

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant LMWorker
  participant UnderstandWorker
  participant request_to_json
  participant JobStatus
  LMWorker->>request_to_json: Serialize result
  request_to_json-->>LMWorker: Return JSON or empty string
  LMWorker->>JobStatus: Mark FAILED if result is empty
  UnderstandWorker->>request_to_json: Serialize result
  request_to_json-->>UnderstandWorker: Return JSON or empty string
  UnderstandWorker->>JobStatus: Mark FAILED if result is empty

Loading

Suggested reviewers: serveurpersocom

Merge Risk: ⚪ Minimal · up to b0b2f

No actionable merge-blocking risk remains from the reviewed changes; normal checks can proceed.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 6 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly summarizes the main changes: UTF-8 validation and job failure when result serialization fails.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 6 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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