[vineethsaivs]

What this fixes

ollama create silently rewrites any SYSTEM, TEMPLATE, MESSAGE or LICENSE value that contains a Unicode format character or a non-ASCII space. The model is created with a prompt the user never wrote, and nothing is logged.

SYSTEM می‌خواهم     stored as  میخواهم     ZWNJ removed, which makes it a different word
SYSTEM 👨‍👩‍👧         stored as  👨👩👧       both ZWJ removed, so it renders as three separate emoji
SYSTEM a b         stored as  ab          non-breaking space removed, joining two words
TEMPLATE ...👨‍👩     stored as  ...👨👩      same in templates

The zero-width non-joiner case is the one that worries me most: in Persian, Arabic and Hindi it is an ordinary letter-level character, not decoration, so removing it changes the word.

Why it happens

The guard that appends a rune to the value buffer tests strconv.IsPrint. That guard exists to discard the rune 0 that parseRuneForState returns for a delimiter it has already consumed. But IsPrint is also false for every format rune (ZWJ, ZWNJ, RLM, soft hyphen) and every non-ASCII space (NBSP), so those get dropped from the value as well.

It is not a deliberate "strip invisible characters" policy. U+FE0F (variation selector 16) is IsPrint true and survives today, so two characters of the same class are treated differently. grep -rn IsPrint --include='*.go' . finds this single line, with no comment, test or documentation describing the stripping as intended.

The change

Test !unicode.IsControl(r) instead. That drops exactly what the guard is for and leaves content alone:

rune	IsPrint (before)	IsControl (after)	written before	written after
rune 0 sentinel	false	true	no	no
\n, \t, \r	false	true	no	no
BEL U+0007	false	true	no	no
ZWNJ U+200C	false	false	no	yes
ZWJ U+200D	false	false	no	yes
NBSP U+00A0	false	false	no	yes
RLM U+200F	false	false	no	yes
soft hyphen U+00AD	false	false	no	yes
VS16 U+FE0F	true	false	yes	yes
ASCII space	true	false	yes	yes

The rune 0 sentinel is itself a control rune, so it is still dropped without needing a separate check. Newlines and tabs are unaffected: they were dropped by this guard before and still are, and multi-line values receive them from the stateValue branch above, not from here.

The stdlib unicode import needs an alias because the file already imports golang.org/x/text/encoding/unicode. strconv had no other use in the file, so its import goes.

Tests

TestParseFileZeroWidthAndNonBreakingRunes, table-driven next to the existing TestParseMultiByte, covering ZWNJ, ZWJ, NBSP, RLM, soft hyphen and the variation selector, plus a case pinning that control characters are still dropped so the change stays scoped.

go test ./parser/          ok
go test ./cmd/             ok

Before the change, 5 of the 7 subtests fail. The two that pass either way are the deliberate controls: the variation selector already worked, and control characters must keep being dropped.

Note on overlap

I have another open PR, #17734, that touches parser.go in a different place (the CRLF line counting near the top of ParseFile). They do not overlap in parser.go, but both append to parser_test.go, so whichever lands second may want a trivial rebase there. Happy to do that whenever.

[HDPark95]

Verified at bcddd84: five new cases are load-bearing (reverting parser.go fails them), but variation_selector passes on main too, as U+FE0F is already strconv.IsPrint.

Untested regression: the guard runs in every state, and stateNil admits any rune into stateName, so an invisible rune starting a directive line now fails the whole file where main ignored it.

FROM m
<U+00A0>SYSTEM hi  ->  command must be one of ... (line 2)

Same for U+200B, U+200C, U+3000, U+2009 and a mid-file U+FEFF. Gating the new rule on curr == stateValue restored all of them, tests still green.

[vineethsaivs]

You are right on both counts, and the regression is a real one I had missed. Fixed in d33fb7c9.

The regression. I reproduced all six of your cases against the branch and they failed exactly as you described, while main parses them fine:

                                 branch bcddd84b            main
NBSP before SYSTEM               ERROR: command must be...  OK  system="hi"
ZWSP before SYSTEM               ERROR                      OK  system="hi"
ZWNJ before SYSTEM               ERROR                      OK  system="hi"
ideographic space before SYSTEM  ERROR                      OK  system="hi"
thin space before SYSTEM         ERROR                      OK  system="hi"
mid-file BOM                     ERROR                      OK  system="hi"

The cause is what you said: the guard runs in every state, so with stateNil admitting any rune into stateName, the invisible rune became part of the command name.

Your suggested gate is what I took. The rule is now relaxed only when curr == stateValue, and strconv.IsPrint stands everywhere else, so command names, parameter names and message roles behave exactly as they do on main. All six cases parse again and the original fix still holds for values.

Added TestParseFileInvisibleRuneBeforeCommand with those six runes, so this cannot come back. It is load-bearing against the mistake rather than just passing: all six subtests fail against the previous commit on this branch and pass on the new one.

On the variation selector, also correct: U+FE0F is already strconv.IsPrint, so that case passes on main too. I meant it as a control, showing that the old rule treated two characters of the same class differently, but the test did not say so. It now sits under a comment marking it and the control-characters case as exactly that, so nobody reads them as evidence of the fix.

go test ./parser/ and go test ./cmd/ are green. Thanks for catching this, it would have been a bad one to land.