[askalf]

Summary

The subtitle and translation services parsed the LLM's reply as JSON only after stripping a Markdown code fence, so a reply with conversational text before or after the object, or a trailing comma, failed the whole step. A new util.ExtractJSONObject finds the object and tolerates those shapes; both call sites use it. Refs #291.

Change risk level

• P0 — Low risk
• P1 — Medium risk
• P2 — High risk: the change is in the Go runtime (runtime/krillinai, internal/service and pkg/util), so it falls under Runtime by this list, although it is +111/-4 lines of production code with no protocol, database, process or configuration change.

Verification

• Targeted checks for the changed files / closest tests
• pnpm typecheck for affected packages (TypeScript changes)
• Module tests for affected areas (P1+)
• Production build (only when touching compile boundaries, lazy loading, assets, or build config)
• Service restart + health check (P2 daemon/runtime changes)
• Web/Desktop parity checks (shared UI, Bridge, or packaging changes)

Actually ran:

go test ./pkg/util/ ./internal/service/ in runtime/krillinai (go1.26.8), on a base arm (the old code-fence stripping) and on this branch: TestExtractJSONObject (21 rows) and the service tests fail on base and pass here. gofmt -l and go vet on both packages: clean.

Intentionally skipped, and why:

No TypeScript changed, so no pnpm typecheck. No compile boundary, asset or build config changed, so no production build. Service restart and health check: not run here; the change is a parsing helper with no daemon lifecycle, protocol or configuration change.

Web / Desktop parity

• This change does not affect Web/Desktop parity
• General capability implemented once in shared Web/Daemon code (no per-bridge forks)
• Platform-specific entries are hidden when the capability is unavailable (no silent no-op buttons)
• The same flow was checked on the other platform, not only the one where the issue was reported

Checklist

• Focused change — no unrelated refactors or fixes mixed in
• Docs updated in the same PR if user-visible behavior changed (no user-visible behaviour changed; the step now succeeds on replies it used to reject)
• No .runtime/, credentials, Codex sessions, build caches, or user data committed

---

Summary

• Adds util.ExtractJSONObject(response, requiredKey) (runtime/krillinai/pkg/util/json.go): strips Markdown fences, then walks the complete JSON values in the response in order and returns the first object that carries requiredKey at top level, dropping a trailing comma before }/] outside string literals.
• Repoints the four LLM-response parse sites that previously fed util.CleanMarkdownCodeBlock straight into json.Unmarshal, each passing the field its prompt asks for: audio2subtitle.go:1875 (align), audio2subtitle.go:1914 (short_sentences), translate.go:142 (short_sentences), translate.go:727 (translations).
• Fixes upstream issue bug: JSON parsing failures in audio2subtitle.go due to LLM conversational text and trailing commas #291: local models (llama3.1 via Ollama) wrap JSON in conversational text and emit a trailing comma before the closing token. Both make encoding/json fail, which costs three retries and then an empty or original result.
• When no candidate carries the field, or the structure is incomplete, the input is returned unchanged (whitespace-trimmed and de-fenced). The caller still sees the real parse error and runs its existing retry.
• 32 test cases at head d2791b2: one 21-row table, TestExtractJSONObject in pkg/util/json_test.go, plus 11 internal/service cases that drive the four call sites through the repo's own scriptedCompleter. 27 fail against upstream master and 5 are declared controls. Every one of the sixteen mutants under Mutation evidence is killed by a named row.

$ git rev-parse HEAD
d2791b22c38ed24ca025eca1f65d6f29195ec273
$ # BASE ARM (ExtractJSONObject stubbed to `return CleanMarkdownCodeBlock(response)`, the upstream master behaviour)
$ go test ./pkg/util/ -run TestExtractJSONObject -count=1 -v 2>&1 | grep -cE '^    --- FAIL'
17
$ go test ./internal/service/ -count=1 -v 2>&1 | grep -cE '^--- FAIL'
8
$ go test ./internal/service/ -count=1 2>&1 | tail -1
FAIL	krillin-ai/internal/service	13.574s

$ # HEAD ARM (d2791b2)
$ go test ./pkg/util/ -run TestExtractJSONObject -count=1
ok  	krillin-ai/pkg/util	0.009s
$ go test ./internal/service/ -count=1
ok  	krillin-ai/internal/service	3.008s

Decisions

The change uses the project’s existing implementation path and adds a focused regression test; broader unrelated behavior remains out of scope.

Not run: no checks beyond the staged PR evidence were run during submission.

AI assistance: this bug was found and the fix and tests were drafted with AI tooling in my workflow; the tests and checks above were executed as pasted. I'm responsible for the change and will handle review feedback.

[wulien]

Correction: I withdraw my earlier request to retarget this PR from master to develop. Targeting master is acceptable; we can integrate the accepted head into develop ourselves. My earlier branch-based objection should be disregarded. Thanks for the focused JSON extraction and service-level regression cases. PR #334 overlaps this fix, so we will review the implementations on their merits and validate whichever integration we select.