Skip to content

fix(compile): use rune slice to truncate history summary — avoids UTF-8 boundary corruption#75

Open
mpeter wants to merge 1 commit into
unbound-force:mainfrom
mpeter:fix/compile-utf8-rune-slice
Open

fix(compile): use rune slice to truncate history summary — avoids UTF-8 boundary corruption#75
mpeter wants to merge 1 commit into
unbound-force:mainfrom
mpeter:fix/compile-utf8-rune-slice

Conversation

@mpeter

@mpeter mpeter commented Jul 9, 2026

Copy link
Copy Markdown

Root cause

buildCompiledArticle in tools/compile.go truncates learning content for the history table using a byte slice:

summary = summary[:80] + "..."

In Go, string[i:j] slices by bytes, not characters. When a multibyte UTF-8 character (e.g. em-dash U+2014, encoded as 0xE2 0x80 0x94) spans the byte-80 boundary, the slice cuts the character mid-encoding. Appending "..." then places 0x2E (period) as the third byte of the incomplete sequence, producing 0xE2 0x80 0x2E — invalid UTF-8.

Fix

Convert to []rune before slicing, then back to string:

if runes := []rune(summary); len(runes) > 80 {
    summary = string(runes[:80]) + "..."
}

Reproducer

Any learning whose information field contains a multibyte character (em-dash, curly quote, arrows, CJK, etc.) where the UTF-8 encoding spans byte offset 80. Observed in production on the process-git compiled article — every compile produced 0xE2 0x80 0x2E causing UnicodeDecodeError in Python consumers of the article.

Testing

go test ./tools/...  # 438 tests pass

@yvonnedevlinrh yvonnedevlinrh 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.

Good catch, the byte-slicing bug is a subtle one and this is exactly the right fix. The writeup in the PR description is really helpful too, appreciate the detail.

Approved.

I'll create a follow-up issue to add a regression test covering multibyte truncation, just so we have coverage if this area changes down the line.

@jflowers jflowers 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.

Good fix — the rune-slice approach is the correct solution for UTF-8 safe truncation.

Two notes for next time:

  1. Use an OpenSpec for bug fixes. Even single-file fixes benefit from the lightweight openspec/changes/ workflow — it creates a traceable record (proposal + tasks), and the tasks artifact is where the regression test requirement (TC-006) gets captured before implementation begins. Running /opsx-propose takes a couple of minutes and would have caught the missing test gap during planning rather than during review.

  2. Regression test (TC-006). Convention pack rule TC-006 [MUST]: bug fixes must include a regression test. @yvonnedevlinrh has committed to a follow-up issue for this — please ensure it's filed and linked.

This review was generated by /review-pr (AI-assisted).

@yvonnedevlinrh

Copy link
Copy Markdown
Contributor

Created #81 for the following on issue of adding in regression testing

Byte-slicing a string at a fixed offset (summary[:80]) corrupts multibyte
UTF-8 characters that span the boundary. For example, an em-dash (U+2014,
encoded as 0xE2 0x80 0x94) sliced at byte 79 or 80 produces an invalid
sequence 0xE2 0x80 followed by the first byte of '...' (0x2E), yielding
0xE2 0x80 0x2E — invalid UTF-8.

Fix: convert to []rune before slicing, then convert back to string.
This preserves character boundaries regardless of multibyte encoding.

Reproducer: a learning whose information field contains an em-dash at or
near byte offset 80 in UTF-8 encoding.

Tested: go test ./tools/... passes (438 tests), recompile of process-git
tag produces clean UTF-8 output.
@yvonnedevlinrh
yvonnedevlinrh force-pushed the fix/compile-utf8-rune-slice branch from fbb7aa4 to 570c2ad Compare July 16, 2026 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review 👀

Development

Successfully merging this pull request may close these issues.

3 participants