Skip to content

feat(translation): render-time translation middleware — pooled endpoints, AIMD pacing, streaming segmentation - #696

Closed
yunsheng111 wants to merge 32 commits into
xintaofei:mainfrom
yunsheng111:feat/translation-middleware
Closed

feat(translation): render-time translation middleware — pooled endpoints, AIMD pacing, streaming segmentation#696
yunsheng111 wants to merge 32 commits into
xintaofei:mainfrom
yunsheng111:feat/translation-middleware

Conversation

@yunsheng111

Copy link
Copy Markdown

Summary

Adds a render-time translation middleware that translates agent output (reply prose and thinking blocks) into the UI language as messages render — without touching the agent, the prompt, or the session files. Closes #695.

  • Streaming segmentation: replies are translated in ~600-char tail-window segments while still streaming; the display chain (translated prefix + raw tail) stays contiguous end-to-end, including across retried/given-up gaps and instance re-keys.
  • Endpoint pool with AIMD: multiple OpenAI/Claude/Gemini/Ollama-compatible endpoints rotate per request; each carries an AIMD rate controller (429 → header-driven cooldown, success → climb), a sampled health score, and per-provider metrics.
  • Quality gates with failover: echo/refusal, answered-not-translated, length sanity, lost placeholders, lost digits, and truncation are all detected and retried on another provider; code spans are placeholder-protected and restored verbatim.
  • Failure handling: consecutive-failure auto-cooldown (threshold & duration are settings, read per request), plus manual per-provider disable / cooldown / reset from the settings table.
  • Two concurrency lanes: priority lane for prose the reader is waiting on, background lane for thinking blocks — caps are settings (1–16).
  • Caching & settings: content-addressed memory+disk cache; settings saves broadcast to every open frontend; selection translation with failure reasons.
  • Untranslatable segments (no Unicode letters) and target-language-identical segments land as identity pieces without spending a request.

Structure

  • Backend src-tauri/src/translation/: pool.rs (rotation, lanes, cooldowns), aimd.rs, health.rs, metrics.rs (per-provider counters, 10-column stats, 6h series), client.rs (protocol adapters, gates), settings.rs (validation, change broadcast), cache.rs; wired into both desktop commands and the Axum web router.
  • Frontend: use-translated-text.ts / use-streaming-translated-text.ts (segmentation, display chain, piece store with source validation), message-render integration, settings page with pool table + trend chart; i18n keys in all 10 locales.
  • Also includes: mermaid call-flow diagrams for the middleware, and a Windows link-directive fix gating the resource manifest behind tauri-runtime (server-mode test exes need no manifest).

Testing

  • tsc --noEmit: 0 errors
  • cargo check desktop + --no-default-features server: both green
  • vitest: 440 files / 6470 tests passing
  • cargo test --lib: 3647 tests passing

30 commits, each with a detailed rationale in its message; the branch is based on current main.

- missingSourceNumbers now dedupes digit runs before judging, matching
  the backend's !runs.contains(current) so a repeated run cannot inflate
  the missing ratio
- remove the unconsumed STREAM_TAIL_CHUNK_MIN_CHARS export: its 600
  floor contradicted the actual TAIL_MIN_SENTENCE_CHARS=400 floor
- state buildContextPrefix's [n] guarantee honestly: the scaffolding
  lines carry no [n] shape, but an interpolated previous segment that
  does degrades safely via the numbered-parse fallback
- drop the stray third argument from four missingSourceNumbers test
  calls (the implementation takes two; vitest did not typecheck them)
The frontend prepends the previous segment as a read-only reference
block to the same outbound text — the model's term-consistency anchor,
which must keep riding to the endpoint. But the block is not content:
the backend ran every local judgment on that full text, so the
reference's numbers counted as source numbers the model was told not to
output (missing_source_numbers rejected faithful translations in a
loop), its English boilerplate cleared the echo gate's 30-letter bar,
and the already-target-language path returned the block verbatim.

Strip the block once at the entry of translate_with_cache and use the
body for skip detection, the cache key, the quality-gate source, and
the cache insert — while the endpoint request still carries the full
outbound. A stub-endpoint regression test pins the wire shape: the
model sees the reference block, the gates do not.
Sweep of the remaining translation-middleware work: backend quality gates
and settings, streaming translation hooks (sentence-boundary chunking,
width-filled batches, bounded settle flush), carry-context reference, and
the frontend wiring/i18n. Includes the settle-flush fix that bounds
re-requests at the nearest reconnectable piece instead of re-translating
already-covered content.
The auto-commit hook swept .ccg/ task state, .zcode/ session plans, a
stale runtest script, and empty scratch outputs into the WIP commit.
Untrack them, ignore the paths, and drop the debug-only probe test
(console.log assertions) from tracking; the probe file stays on disk.
…tity release

Three layers against deterministic echo/answer failures on unreliable
endpoints: every outbound body rides a <translate> envelope (source as
DATA, separate from instructions); retries escalate a constraint line
because an identical request at temperature 0 returns an identical wrong
answer; and a settle gap whose retry budget is spent stitches the chain
with an identity piece instead of freezing every translated piece behind
it (display store only — never cached as a translation).
The backend strips the <translate> envelope for skip detection, the
cache key, and the quality gates (same contract as the reference-block
strip), while the full outbound still rides to the endpoint; the system
prompt names the envelope so models respect the boundary.
…its retry

Failed regions outlive the hook instance that recorded them: a
pendingGapsStore keyed alongside the piece store records every unlanded
segment (dispatch rollback, failed settle flush), and a replay effect
re-dispatches recorded gaps on mount and re-key — deliberately not
viewport-gated, since the settle-time re-key is exactly when the old
instance's variant-retry chain dies. Backoff and pause budgets keep a
dead endpoint from looping.

Dispatch paths also claim their segments in flight now: the cursor
rollback and text-flush effect re-runs could send the same segment twice
(observed once with the carry-context reference, once without), spending
quota twice for one translation.
An endpoint that returns the source unchanged used to slip past the
echo gate whenever the chunk masked down to placeholders plus a few
words — the gate's >=30-letter prose bar never fired on code-heavy
thinking text, and the echo was served as a "translation" (observed
three times on one relay with no rejection recorded).

The new exact-echo gate compares content instead of script coverage:
placeholders stripped from both sides, whitespace normalized, reply
equal to source means echo. Placeholder-only chunks stay exempt —
echoing [[CBLK0]] back IS the correct translation. Mirrored on both
sides (frontend judge, backend gate) with the same normalization, so
one reply cannot pass one side and fail the other.
… flight

An endpoint answering in 76-112 s used to hold a Background-lane slot
for the whole round trip and only reached the health window when the
reply landed — three such requests stalled the entire thinking
translation lane for over a minute while the reader stared at raw text.

The soft in-flight deadline (30 s) now records a SlowInflight event the
moment the wait becomes reader-visible and halves the provider's rate
on the spot, so later chunks rotate elsewhere while the slow request is
still out. The event weighs half on stability, like a 429 — nothing
failed, but the lane is being held — and is deliberately excluded from
the speed window, where the eventual reply records the true round trip
(counting both would double-punish one ask). Quality never hears about
it: no reply was judged.
An endpoint that answers a wide chunk with a self-written essay (the
observed shape: a 423-character block returned as a 1600-character
document) was refused and re-requested whole — at temperature 0 the
retry found the same rope and wrote the same essay. An invention-shaped
rejection on a chunk over 800 characters now buys one split: the chunk
goes back out as two halves judged independently, narrowing the space
the model can wander in. Any other rejection, a short chunk, or a half
that fails again keeps the original verdict.

Splitting exposed a latent splitter bug: the strong sentence-end set
omitted the ASCII period, so English — the primary source language —
never cut at sentence boundaries anywhere (tail chunks, stable-unit
splits, settle flushes all degraded to whitespace-at-window-edge). The
half-split boundary now lands on real English sentence ends too.
Reconstructing which UI block produced which backend request from a
mixed traffic log meant matching on text prefixes. The blockKey now
rides the translate call end to end — cache key first field, through
both IPC surfaces, into the client's sending/response/failure lines —
so "this paragraph failed three times on this endpoint" is one grep
away.
…ly comes through

The final review caught the half-split firing on the wrong door: the
backend's length gate rejects the reply before the frontend judge ever
sees it, and the observed production invention (423-character chunk,
1600-character document) arrived exactly that way — result.error, never
judged — so the split never fired for it. An invention-shaped
result.error now triggers the split on both the lone-chunk and the
per-chunk fallback path; the judge-side trigger keeps only the
INVENTED_CONTENT code (the 'far longer' text match there was dead —
fixed codes only) and the join now restores the boundary space a
same-line split consumes.

Also: deterministic placeholder/surrogate straddle tests (the old
assertion never exercised the move), and a comment on why the dispatch
cursor may cover a still-inflight segment.
- A segment with no Unicode letter (separators, symbol runs, digit-only
  fragments) maps to itself in every language, so it now lands as an
  identity piece instead of a request that can only be echo-refused.
  One content rule covers every decoration without a symbol whitelist.
- A gap refused by three consecutive replays is dropped instead of being
  replayed through every widening backoff forever: the raw source stays
  displayed and the concurrent slot is freed. The count is keyed by
  content, so a re-keyed instance cannot resurrect the region.
A reply that settled just as the block re-keyed was dropped by the
isCurrent check: neither the store nor the state got it, and the
replacement instance re-requested the segment from scratch. Observed in
the 02:49 session — the same thinking-tail segment was fetched three
times, each reply arriving fine, each discard re-queueing it; the
display stayed raw for the whole turn.

All three landing paths (streaming land, gap replay, settle flush) now
merge their pieces into the store synchronously before the gate. The
content-addressed restore hands them to whoever takes the key next, so
a re-key costs a re-render instead of a re-request.
The 04:14 session showed a reply whose FIRST paragraph was Chinese (the
model's preamble under a global zh convention) while the rest was
English. Translating the preamble to zh returns it verbatim, the echo
gate refused it, and the gap was eventually given up — leaving no piece
at offset 0. The display chain breaks at the first byte, so all thirteen
translated paragraphs behind it rendered as raw English under a toggle
claiming the block was translated.

Two fixes plus a race found while reproducing:

- Giving up a gap now stitches the raw source as an identity piece (the
  settle flush's whitespace stitch, extended to give-ups): the chain
  stays whole and the raw lead renders ahead of the translated rest.
- A segment already written in the display language (Han-dominant
  against a zh target, judged on settings.targetLang ?? uiLocale) lands
  as an identity piece without a request — the echo it would get back IS
  the correct translation.
- The settle flush now also holds off while any in-flight SUBSEGMENT of
  its gap is on the wire: a flush request sent past the fallback landings
  could be refused and given up, stitching the whole span raw and burying
  the subsegment's translation under an identity piece the chain can
  never see past. This replaces the accepted one-shot double-spend: the
  flush re-runs on the landing and spends one request, not two.
Four mermaid diagrams over the translation middleware: the end-to-end
call chain (frontend hooks -> backend pool -> endpoint), the streaming
segmentation and display-chain assembly, the quality gates with retry
escalation, and a full lifecycle sequence for one reply. Edge labels
validated against the mermaid parser the app itself embeds.
The old behavior dropped every line after the daily 512MB ceiling until
the UTC rotation, which silenced exactly the logs a user debugging a
problem was looking at. Crossing the ceiling now truncates the day's
file and resumes with a fresh budget — the disk stays bounded and the
log never goes quiet. A failed truncation drops its line and retries on
the next crossing. Verified live: a file parked at the ceiling was cut
to zero on the first line of a fresh process.
A provider that keeps failing stayed in the rotation until its client-
error count benched it for the whole session — too blunt for a flaky
endpoint and irreversible without a restart. Providers can now be
parked and revived explicitly, and the auto-cooldown is a setting
instead of a hardcoded session exile:

- Three new commands (and web routes): provider_reset, provider_disable,
  provider_cooldown. The settings page renders a Ban/Timer/RotateCcw
  action per provider row plus a header strategy popover.
- Consecutive failures (429s and hard errors alike) now trigger a
  bounded cooldown — failure_threshold (default 3) and cooldown_seconds
  (default 60) live in settings, read per request so a mid-flight edit
  cannot rewrite the thresholds under an in-flight pick.
- Two lane concurrency caps: priority_max_concurrent for prose the
  reader is waiting on, background_max_concurrent for thinking-block
  translation so it cannot crowd out the priority lane on a small
  endpoint.
- translate_body setting (default true) so reply-prose translation can
  be turned off while thinking blocks keep theirs.
- Saves broadcast translation-settings-changed to every open frontend;
  other windows re-fetch instead of keeping the mount-time snapshot.
- New i18n keys across all 10 locales; settings tests cover the new
  UI surface.

Housekeeping: gate the Windows resource link directive behind
tauri-runtime (server-mode test exes need no manifest), ignore
security-scan artifacts.
# Conflicts:
#	src-tauri/src/lib.rs
#	src/components/message/message-list-view.tsx
#	src/i18n/messages/ar.json
#	src/i18n/messages/de.json
#	src/i18n/messages/en.json
#	src/i18n/messages/es.json
#	src/i18n/messages/fr.json
#	src/i18n/messages/ja.json
#	src/i18n/messages/ko.json
#	src/i18n/messages/pt.json
#	src/i18n/messages/zh-CN.json
#	src/i18n/messages/zh-TW.json
@xintaofei

Copy link
Copy Markdown
Owner

Review

First — thank you for this. It's an enormous piece of work, and it shows: the commit messages are some of the best I've seen in this repo (each one explains why, with the observed failure that motivated it), the i18n is genuinely complete (145 keys × 10 locales, real translations, no English fallbacks), the streaming tests describe real races rather than happy paths, and masking code spans out of the request before it leaves the machine is exactly the right call. The problem is real too — reading long English agent output while switching to a translation tool does break the flow, and "don't touch the agent, the prompt, or the session files" is the right constraint to build under.

I want to be equally straight with you about where it stands: I can't merge it as it is. There are four blocking issues, three of which make CI red, plus a set of correctness problems in the gate/cache/pool interaction. I've written all of them up below with reproductions so none of it is guesswork, and there's a section at the end on how I'd suggest splitting this up — because I think the biggest issue isn't any single bug, it's that this is three PRs' worth of surface area in one.

What I ran

Check Result
tsc --noEmit ✅ clean
pnpm eslint . ✅ (1 pre-existing warning in an untouched file)
cargo clippy --all-targets --features test-utils -- -D warnings ✅ clean
cargo test --features test-utils exit 101
pnpm test exit 1

The last two are the repo's actual gates (.github/workflows/test.yml runs pnpm test; src-tauri/CLAUDE.md documents cargo test --features test-utils). The PR body lists cargo check and cargo test --lib, and that gap is exactly where two of the blockers hide — neither of those commands links an integration-test binary.


Blocking

B1 · cargo test --features test-utils can't build on macOS or Linux

src-tauri/build.rs:21-30

The new cargo:rustc-link-arg-tests=<OUT_DIR>/resource.lib is gated only by #[cfg(feature = "tauri-runtime")]. resource.lib is a Windows-only tauri-build artifact, so everywhere else the linker gets a path that doesn't exist:

clang: error: no such file or directory: .../out/resource.lib
error: could not compile `codeg` (test "backup_api" | "delegation_columns"
       | "delegation_e2e_uds" | "log_file_budget" | "macos_icon_geometry" | "parsers_snapshot")

find src-tauri/target -name resource.lib returns nothing on macOS, and git show main:src-tauri/build.rs has no such directive — so this is new. The frustrating part is that you got the other half of the same fix right: the #[link(name = "resource")] in src-tauri/src/lib.rs:23-29 is properly gated on target_os = "windows", target_env = "msvc". Only the build-script side was left open.

Fix: one guard, but not the obvious one — in a build script #[cfg(target_os = ...)] describes the host, not the target, so it would break cross-compiles. Read the env instead:

let windows = std::env::var("CARGO_CFG_TARGET_OS").as_deref() == Ok("windows")
    && std::env::var("CARGO_CFG_TARGET_ENV").as_deref() == Ok("msvc");
if windows {
    println!("cargo:rustc-link-search=native={out_dir}");
    println!("cargo:rustc-link-arg-tests={}", resource.display());
}

B2 · A corrected API key never reaches the endpoint — and "Test connection" passes anyway

src-tauri/src/translation/pool.rs:303-332

pool_key() is built only from chat_completions_url()|model, and pool_for() uses entry(key).or_insert_with(...). On a key hit the freshly-loaded providers vector — carrying the new api_key, rpm_cap, name — is dropped on the floor, and the cached PoolState holding the original ProviderConfig is returned. pick_excluding hands that stale config out and translate_one signs the request with provider.api_key. Nothing invalidates the POOLS registry on save.

I confirmed it with a throwaway test (since reverted) — two pick_provider() calls with the same base URL + model but different keys:

PROBE: after saving NEW-KEY the pool hands out: "OLD-KEY"

The user-facing shape is what worries me most. Someone typos their key, gets 401s, fixes it, saves, clicks Test connection — which succeeds, because pool::standalone() uses the form's provider — and then translation keeps 401ing anyway. Reset doesn't rescue them either: reset_provider re-seeds from pool.providers, i.e. the same stale config. Only editing the base URL/model or restarting the app recovers. That's a very hard bug for a user to reason about.

Fix: when the incoming list differs, refresh the entry's providers in place (keeping the runtime map so the adaptive history survives), or fold a config fingerprint into pool_key. The first is nicer — you don't want to throw away AIMD learning just because someone renamed a row.

B3 · pnpm test is red, in test files this PR never touched

src/hooks/use-translated-text.ts:148-160, reached from src/components/message/content-parts-renderer.tsx:2266 and :3037

Rendering any message part now mounts useTranslationSettingsSnapshot()ensureSettingsLoaded()bindSettingsChangeEvent()void subscribe(...)getShellTransport()require("./web-transport"), which throws under vitest. That promise has no .catch(), so it becomes an unhandled rejection — and vitest exits non-zero on unhandled errors even when every assertion passes:

$ pnpm vitest run src/components/message/collapsible-system-message.test.tsx
 Test Files  1 passed (1)   Tests  3 passed (3)   Errors  1 error     → EXIT=1

That file is pre-existing and untouched by this PR. To pin the attribution exactly I inserted an early return before the void subscribe(...) line, re-ran the same file → EXIT=0, 0 errors, then reverted. Full suite: Test Files 4 failed | 435 passed, Errors 6, exit 1.

To be fair to you: those 4 "failed files" are 5 s-timeout flakes under CPU load, not your regressions — re-run alone, only translation-settings.test.tsx still needed --testTimeout=30000, after which all 32 pass. The 6 unhandled rejections are the real signal.

Two related things while you're in there: settingsEventBound = true is set before the subscribe can fail, so a failure is never retried; and translation-settings.tsx:530 has the same uncaught pattern.

Fix: .catch() the subscribe promise, and — more importantly — hoist the settings snapshot + subscription out of the per-part render path into one app-level provider. A leaf render component shouldn't be the thing that opens a transport subscription.

B4 · The bundled logging change breaks an existing integration test

src-tauri/src/logging/budget.rs (commit a2bb097f) vs src-tauri/tests/log_file_budget.rs:101

That test asserts total > SEEDED. Under the new "truncate the day's file at the ceiling" behaviour the file is zeroed and only post-truncation lines remain, so the total drops below the seed and the assertion fails — measured at 24,510 bytes against a 24,576-byte seed, with B1 worked around so the test could link at all. The test was never updated, and because of B1 it can't be compiled on macOS/Linux, so there was no way you'd have seen it.

Separately, budget.rs:263-276 sets self.written = len for the crossing line, so a single oversized record lands past the ceiling — which contradicts the comment directly above it ("a single huge line can't jump the ceiling and land on disk anyway").

I'd honestly rather this change came out of this PR entirely — see the scope note at the bottom.


Important (not blocking the build, but I'd want these before merge)

I5 · Two "already in the target language" predicates that disagree — producing a retry loop that can never win

Backend translation/mod.rs:343-388 uses every non-whitespace char as the denominator and counts CJK punctuation (U+3000–303F, U+FF00–FFEF) as CJK. Frontend lib/translation.ts:345-360 counts only letters, only Script=Han, and needs han*2 > letters.

Worked example — 先执行、再检查、最后合并。命令是 git rebase main。

  • frontend: letters = 26, han = 13 → 26 > 26 is false → sends the request
  • backend: non-ws = 30, cjk = 13 + 4 punctuation = 17 → 0.567 > 0.5 → skips the endpoint, returns the body verbatim with error: None
  • frontend: echoVerbatimError(chunk, chunk, "zh-CN")ECHO_VERBATIM → discard → retry with an escalated constraint variant

…which cannot possibly help, because the backend short-circuits before the endpoint. The retry gets the identical answer every time until the retry and give-up budgets run out, the region is stitched raw, and the block wears the amber "Some chunks failed to translate" badge.

It's worse for ja/ko: the frontend returns false for any non-zh locale (line 350), while the backend handles both — so for a Japanese or Korean UI this fires on every already-target-language segment.

This is the same lesson normalize_echo_text already learned ("the two gates must agree or one reply passes one side and fails the other") — it just wasn't applied here. The cleanest fix is to stop having two: have the backend flag the identity path (skipped: true) so the frontend bypasses its gates for that slot.

I6 · The cache defeats the placeholder retry entirely

Placeholder validation is deliberately frontend-only (mod.rs:126-135), but the backend caches after its own three gates (mod.rs:541), keyed on the envelope-stripped body. The retry's constraint line rides outside the envelope (use-translated-text.ts:454), so once stripped the key is byte-identical → cache hit → the same damaged reply comes back.

So PLACEHOLDERS_LOST can never be recovered by retrying, no matter how many variants escalate. The whole retry-shape mechanism is inert for the one gate the backend doesn't replicate.

I7 · One reply is recorded as both a success and a failure

client.rs:609-613 records ProviderEventKind::Ok and calls report_success() — which clears the consecutive-failure streak and rewards AIMD — for any reply that merely parses. mod.rs:517-524 may then gate-reject that same reply and call record_gate_rejection.

Net effect: an endpoint that reliably returns well-formed garbage can never trip the failure cooldown, and its health window sees two events per attempt. That's precisely the failure mode health.rs was built to catch, so it's worth untangling — the accept/reject decision needs to happen before anything is reported.

I8 · Health retirement can disable the last viable endpoint

pool.rs:492-496 computes complete_count from all complete providers, including ones already session-disabled or excluded for this request. With two complete rows where one is already disabled, complete_count = 2 > 1 lets the survivor be retired too, leaving "All translation endpoints are disabled for this session" — the exact outcome the single-point guard was written to prevent.

I9 · Abandoned pacing reservations can wedge a provider

wait_for_dispatch_slot (pool.rs:266-287) writes entry.last_dispatch_us = slot before it sleeps, and translate_batch join_alls every text eagerly (client.rs:1017). Nothing caps texts.len()mod.rs:415-423 caps each text individually only. So a large batch (or one that gets cancelled) can push a provider's timeline hours ahead, and dropping the futures doesn't undo the reservations. A texts.len() cap plus releasing the slot on drop would close both halves.

I10 · The Anthropic dialect skips half the reply-side defenses

client.rs:861-887 applies neither strip_reasoning_block nor refusal_shape, both of which the OpenAI path applies. An Anthropic-compatible relay fronting a reasoning model will have its <think>…</think> served as the translation, and a refusal isn't caught at parse time. Worth factoring the post-parse pipeline (strip reasoning → refusal check → empty check) into one function both dialects call.

I11 · Disk cache: full rewrite per insert, full parse under the lock, lossy under concurrency

cache.rs:270-293 reads, deserializes, linearly scans, reserializes and rewrites the entire per-language file (MAX_DISK_BYTES = 10 MiB) on every insert — and a streaming reply inserts once per segment. cache.rs:164-179 does the same full read+parse on a memory miss while holding the global mutex, so every lookup in the process serializes behind it. Both are blocking I/O on the tokio runtime with no spawn_blocking.

The mutex is also released before the read-modify-write, so concurrent inserts can lose each other or produce malformed JSON, and clear() can race a persist that recreates the file it just deleted. File::create truncates before writing, so a crash mid-write loses the language file.

Append-only JSONL with periodic compaction (or SQLite, already a dependency) would fix all of it at once.

I12 · Cache keys no longer include the provider, contradicting the module's own doc

cache.rs:140-161 promises "provider_id participates so switching endpoint or model never serves output produced by the previous one" — but TranslationSettings::provider_id() (settings.rs:446-448) returns the constant "pool". So switching endpoint or model keeps serving the old one's translations from disk.

That matters most in exactly the scenario the whole gate stack exists for: "this endpoint translates badly, let me switch". primeTranslationSettings clears only the frontend map, so the re-request just hits the stale backend cache — the clearing is cosmetic. And Clear cache clears neither translatedCache nor pieceStore, so it doesn't visibly clear anything already on screen either.

I13 · Translated pieces aren't scoped to the target language

use-streaming-translated-text.ts:111 validates restored pieces on source bytes alone. Switch zh-CN → ja and the Chinese pieces keep validating and rendering indefinitely. primeTranslationSettings clears translatedCache/translationInflight but not pieceStore/pendingGapsStore.

I14 · Everyone pays for translation whether or not they enabled it

use-streaming-translated-text.ts:495-498 computes segmentsFor(text, …) with no enabled guard. That runs splitStableUnits (full line scan) plus tailChunksForsentenceChunkEndunclosedBrackets. TextPart calls the hook for every assistant text part, and text changes on every streaming token — so it's a full scan per token over a growing string, cumulatively quadratic, for the large majority of users who will never turn translation on. ReasoningPart additionally mounts two hooks plus an IntersectionObserver per block.

Given how much perf work has gone into the message list, this one I'd really like fixed before merge. Gating at the call site ({settings.enabled ? <TranslatedTextPart/> : <PlainTextPart/>}) is the cheap version.

I15 · useTranslatedText's React hook is inert at its only call site

content-parts-renderer.tsx:3036-3055: when p6Enabled is true the hook is disabled; when it's false the hook's internal enabled (settings.enabled && settings.translateThinking, i.e. p6Enabled) is false and shouldTranslate short-circuits. So it can never issue a request — yet every reasoning block mounts it.

(To be precise: the module isn't dead — requestTranslationDetailed, translationCacheKey and useTranslationSettingsSnapshot are all load-bearing. It's the render hook, and the 609 lines of tests describing its behaviour, that exercise a path the product never reaches.)

I16 · test_connection can silently test a different provider

client.rs:1045-1052: if the requested row isn't is_complete(), it falls back to pick_provider(settings). So an incomplete backup row can be reported healthy because a different row answered.

I17 · Error bodies are read unbounded

Success bodies go through read_capped (1 MiB), but client.rs:648 and the model-list path use plain response.text(). A hostile or broken endpoint can force arbitrary buffering; the timeout bounds duration, not memory.


Minor — recording, not asking you to chase

Expand
  • client.rs:139-164lane_semaphore takes lanes.write() unconditionally, while its doc promises a read-lock fast path.
  • pool.rs:880-901standalone()'s doc claims it shares the live pool's runtime map; it builds a single-member key, different from any multi-member pool.
  • client.rs:566record_dispatch runs after the round trip, so dispatched_last_minute counts completions rather than dispatches (its doc says otherwise). A read_capped failure reports to the pool but skips record_attempt. strip_reasoning_block removes only the first <think> block.
  • translationCacheKey puts blockKey first, so "the same paragraph translates once and is reused everywhere" (its own doc) isn't true — a positional re-key misses the frontend cache.
  • Identity/"stitch" pieces make hasTranslation true for blocks whose "translation" is byte-identical to the source, so the 原文/译文 toggle appears on content that was never translated.
  • No integration test for the 12 new /api/translation_* routes, though the repo has 13 integration binaries including api_integration.rs and backup_api.rs. The axum wiring, JSON shapes, key masking over the wire and auth are all untested.
  • The settings copy never says that enabling translation sends every rendered reply as prose to a third-party endpoint. (Storage detail for accuracy: disk holds the translation plus a SHA-256 of the source, not the source itself — but client.rs:487 and :599 log source and response snippets at debug level, which does put message content in the log file.) One sentence in sectionDescription naming the outbound flow — and crediting the fact that code spans never leave — would be a nice touch, since the masking is genuinely a feature.

Scope. A few things rode along that I'd like to separate out:

  • a2bb097f rewrites logging/budget.rs (491 lines) with a destructive behaviour change — crossing the 512 MB daily ceiling now truncates the user's log file. It isn't mentioned in the PR description, it breaks tests/log_file_budget.rs (B4), and it deserves its own review: truncation throws away the older half of the day, which for a crash investigation is usually the half you need. Size-based rotation keeping N files is the conventional answer.
  • 8a96079b also lands three regenerated CLAUDE.md architecture docs. They say "当前版本 0.29.0" (repo is 0.30.6) and — ironically — never mention the translation/ module this PR adds, the translation settings page, or the new endpoints. So they're stale on arrival.
  • .gitignore picked up what look like local tool scratch paths (.ccg/, .zcode/, .mimosa/, .magi/, src/probe.test.ts, src-tauri/runtest.*, bash.exe.stackdump). Those are better in .git/info/exclude or a global gitignore than in the shared file.
  • There's global cargo fmt churn in files the feature doesn't touch — web/router.rs (git_update_branch, acp_goal_control, automation_get, forge_tab_count), lib.rs, bin/codeg_server.rs, tauri.conf.json. We deliberately don't run repo-wide fmt here (no fmt gate), so those hunks just add review noise.

On the shape of it

This part is a judgement call rather than a bug list, and I want to be upfront that it's the harder feedback: I think the feature is worth having, and I think this particular design is bigger and more fragile than the problem needs.

The quality gates have no natural stopping point. Echo detection, twelve hardcoded English refusal openings, digit-run counting, length ratios, script-fraction thresholds, invention split-retry, constraint escalation, an XML envelope, a context reference block — each was added because a specific relay misbehaved, each has a false-positive mode that silently discards a good translation, two of them now contradict each other across the language boundary (I5), and a third is neutralised by the cache (I6). The next misbehaving endpoint adds a fourteenth. The structural fix isn't more gates — it's one authoritative acceptance decision, owned by one side, with cache insertion, metrics and retry classification all downstream of it. That single change would dissolve I5, I6 and I7 together.

Worth at least considering: a purpose-built translation API (DeepL / Google / Azure) doesn't hallucinate, doesn't refuse, and doesn't answer the question the text asks — which is what most of this machinery exists to detect. Supporting one as the default path, with the OpenAI/Ollama path kept for people who want local or self-hosted, would delete most of the gate stack rather than maintaining it.

The pool infrastructure is sized for a service, not a desktop app. AIMD + composite health scoring + a six-hour per-minute metrics series + probe scheduling, all to pace requests to the user's own endpoints. A single-endpoint user — which will be almost everyone — short-circuits both the retire gate and the degrade partition and gets none of the value, while I8 shows the ranking can actively strand them. I'd keep AIMD, the 429 cooldown and the two concurrency lanes (those help even with one endpoint) and drop health ranking, retirement and the trend chart — roughly 1,600 lines that only pay off with several endpoints.

Streaming segmentation is where the risk concentrates. use-streaming-translated-text.ts is 1,474 lines: content-keyed piece stores, pending-gap stores, in-flight claim sets, give-up counters, replay rounds, settle boundaries, three separate identity-stitch paths. Reading the comments, it's clearly a hard-won log of races found the painful way — and I don't doubt any single one of them. But the payoff is modest: what the reader actually sees is a translated prefix plus a raw tail that rewrites itself underneath them. Settled-only translation would delete nearly all of that state machine.

There's no cost control. No per-message "translate this" action and no token budget. Once enabled, useNearViewport latches true permanently, so scrolling back through a long history eventually fires an LLM call for every assistant text part and reasoning block you pass. On a metered endpoint that's a decision the UI never surfaces.

What I'd suggest

Rather than asking you to fix twenty things in a 23k-line branch, I'd much rather land this in pieces:

  1. Settings + single-endpoint client + settled-only translation + cache, with one owner for the accept/reject decision. That's the entire user-visible feature and maybe a third of the code — and I'd merge it happily.
  2. The multi-endpoint pool (round-robin + 429 backoff + AIMD), when there's a user with two endpoints to justify it.
  3. Streaming segmentation, on top of a stable settled path.

And pull the logging rewrite and the CLAUDE.md regeneration out into their own PRs — the log-truncation change in particular I want to look at on its own terms.

If you'd rather keep it as one branch, that's workable too; then B1–B4 plus I5, I6, I7 and I14 are what I'd need before merge, and the rest can be follow-ups.

Genuinely: thank you for the depth of work here, and sorry for the length of this. Happy to talk through any of it — especially the split, if you want to figure out where the seams should go.

@yunsheng111

Copy link
Copy Markdown
Author

Thank you for the unusually thorough review — I agreed with the core diagnosis that this was three PRs' worth of surface area, so I rebuilt it from main along the seams you suggested rather than trying to patch the 23k-line branch in place.

The replacement stack is:

  1. feat(translation): render-time translation core — settings, single-endpoint client, settled-only, cache #700 — settings + single-endpoint client + settled-only translation + cache. This is the upstream-facing PR against main.
  2. feat(translation): rotation pool — AIMD pacing, cooldowns, per-provider stats yunsheng111/codeg#1 — multi-endpoint rotation pool, stacked on feat(translation): render-time translation core — settings, single-endpoint client, settled-only, cache #700 in the fork for now.
  3. feat(translation): streaming segmentation with call-site gating and language-scoped pieces yunsheng111/codeg#2 — streaming segmentation, stacked on the pool PR.

How the review items landed:

  • B1: resource.lib link args are gated by CARGO_CFG_TARGET_OS / CARGO_CFG_TARGET_ENV, not build-script #[cfg].
  • B2: the live pool refreshes provider configs in place (same Arc, runtime/AIMD retained; an API-key correction takes effect immediately, and an rpm-cap change reseeds only that member).
  • B3: subscription rejection is caught, the bound flag is set only after success, and a later mount retries — the full vitest suite now exits cleanly with no unhandled rejection.
  • B4 / scope: the logging-budget rewrite is absent from all replacement PRs, as are the CLAUDE.md regeneration, shared .gitignore tool paths, and unrelated fmt churn.
  • I5/I6/I7: translate_one is now the sole acceptance owner. Gates run before success reporting and before cache insertion; identity results carry skipped: true; retry variant is part of the cache key.
  • I8: removed by construction — the pool has no health ranking or automatic retirement.
  • I9: batch dispatch is windowed and abandoned dispatch-slot reservations are returned on drop.
  • I13/I14/I15: streaming pieces/gaps are target-language scoped, translation hooks are gated at the component call site (disabled users mount none of them), and reasoning uses one real streaming/settled path rather than the inert dual-hook path.
  • I16/I17: test-connection never falls back to another row; error/model-list bodies use capped reads.

The pool PR deliberately keeps multi-provider rotation, AIMD, Retry-After and failure cooldowns, two lanes, manual operations and per-provider counters, while dropping health ranking, retirement, probes and the six-hour trend chart.

Verification was rerun against the actual repository gates rather than the narrower commands from the old PR:

I'll close this oversized PR in favor of the replacement stack. The fork PRs are intentionally based on the preceding fork branch so their diffs stay reviewable; after #700 lands they can be rebased/retargeted upstream one at a time.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

【Feature request】Agent 输出实时翻译中间件:多端点池化 + AIMD 自适应限速 + 流式分段翻译

2 participants