Skip to content

Every round spoke English because System shadowed the submitter - #960

Merged
rbuergi merged 2 commits into
mainfrom
fix/948-round-locale-propagation
Aug 8, 2026
Merged

rbuergi merged 2 commits into
mainfrom
fix/948-round-locale-propagation

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #948.

The defect

A round resolves its user-facing text explicitly off AccessContext.Locale (never ambient culture — a round hops schedulers). But the round-dispatch watcher rebuilds that context from the submitter rider on the pending message, and the rider carried only ObjectId + Name. The language was dropped, so every user-facing string a round emits rendered English for every viewer, whatever their language.

What tracing turned up (one level up from where the issue predicted)

The issue expected the locale to be available at the capture site and simply not carried. It was worse: CaptureSubmitter took the first non-null access context, and at the submit boundary the request-scoped context is frequently a non-user principal — a hub address, or system-security from an enclosing ImpersonateAsSystem scope. A diagnostic printed exactly that:

same=True  ctx=system-security/-  circuit=Roland/de

So the rider was stamped system-security while the real signed-in German user sat on the circuit context. It lost the language and mis-stamped the identity — and note the watcher's own IsRealUser predicate rejects system-security, so the rider was smuggling a principal straight past a gate that exists to stop it.

The fix — both ends of the rider now agree

  • CaptureSubmitter picks the first REAL USER context, not the first non-null one, applying the same predicate the watcher applies (no hub-shaped principals, no System).
  • The rider carries the locale next to the identity (ThreadMessage.SubmitterLocale), stamped by StartThread / SubmitMessage / SubmitComposer.
  • DispatchRound consumes it in BOTH branches. The ambient branch needs it too: the thread hub's OWNER-INJECTION stamps the owner as {ObjectId, Name} only, so an identity taken from there is locale-less by construction.
  • The CreatedBy fallback deliberately stays default, with a comment saying so — a bare user id on the node carries no presentation preference, and inventing one (e.g. reading ambient culture) would be worse.
  • The dispatch identity trace now prints the language, so "renders English for a German user" is diagnosable from the log alone.

The un-vacuumed test

ModelSubstitutionTest.NoUsableModelAnywhere_FailsRoundWithSpeakingError asserted the round's summary against the English catalog entry (locale: null) — which passes equally against a hard-coded English literal. It now submits as a German user and asserts the German text, with a sanity assertion that the EN and DE entries differ so it cannot go vacuous again, plus a direct assertion on the rider so a regression names its own cause.

Before this change (verified on the branch with only the test edit applied):

Expected value to be "Kein Sprachmodell kann diese Runde bedienen: …",
but found "No language model can serve this round: …".

After: Passed! - Failed: 0, Passed: 2 (Release).

Verification

  • ModelSubstitutionTest (both cases): 2/2 passed, Release, on the tree merged with current main.
  • Wider regression for the changed CaptureSubmitter semantics — MeshWeaver.AI.Test --filter ~Thread: 94/94 passed, Release.
  • dotnet build -c Release -warnaserror, one project per invocation: MeshWeaver.AI, MeshWeaver.Documentation, MeshWeaver.AI.Test — all 0 warnings, 0 errors.
  • Branch merged with origin/main before pushing.
  • No new user-visible strings were introduced (the existing chat.noUsableModel* catalog keys are reused), so no strings.{en,de}.json changes are needed; no CultureInfo.CurrentUICulture resolution anywhere in the diff.

Sweep

chat.noUsableModel / chat.noUsableModelNoSelection are currently the only round-emitted strings resolved from userAccessContext?.Locale; both now render in the submitter's language. The fix is generic — any future round-emitted localized string inherits it.

What's New: included (user-visible — round messages now appear in your language).

🤖 Generated with Claude Code

A round's user-facing text is resolved explicitly off the round's
AccessContext.Locale (never ambient culture — a round hops schedulers).
But the round-dispatch watcher REBUILDS that context from the submitter
rider on the pending message, and the rider carried only ObjectId and
Name. The language was dropped on the floor, so every user-facing string
a round emits rendered English for every viewer regardless of their
language — the exact failure mode the i18n rule exists to prevent.

Tracing it turned up the reason the language was missing in the first
place, one level up from where #948 predicted. CaptureSubmitter took the
first NON-NULL access context, and at the submit boundary the
request-scoped context is frequently a non-user principal — a hub
address, or system-security from an enclosing ImpersonateAsSystem scope.
It stamped THAT as the submitter (`ctx=system-security/-`) while the real
signed-in user sat right there on the circuit context (`circuit=Roland/de`).
So the rider lost not just the language but the identity: the watcher's
own IsRealUser predicate rejects system-security, yet the rider smuggled
it straight past that gate.

Fixed at both ends of the rider, which now agree:

- CaptureSubmitter picks the first REAL USER context, not the first
  non-null one — applying the same predicate the watcher applies (no
  hub-shaped principals, no System).
- The rider carries the submitter's Locale next to their identity
  (ThreadMessage.SubmitterLocale), stamped by StartThread /
  SubmitMessage / SubmitComposer.
- DispatchRound uses it in BOTH branches. The ambient branch needs it
  too: the thread hub's OWNER-INJECTION stamps the owner as
  {ObjectId, Name} only, so an identity taken from there is locale-less
  by construction. The CreatedBy fallback deliberately stays default —
  a bare user id on the node carries no presentation preference, and
  inventing one would be worse. Commented as such.
- The dispatch identity trace now prints the language, so "renders
  English for a German user" is diagnosable from the log alone.

The test that should have caught this was vacuous: it asserted the
round's summary against the ENGLISH catalog entry (locale: null), which
passes equally against a hard-coded English literal. It now submits as a
German user and asserts the German text, with a sanity assertion that
the EN and DE entries differ so it cannot go vacuous again, plus a
direct assertion on the rider so a regression names its own cause.
Verified failing before this change (found English, expected German) and
passing after.

Fixes #948.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 8, 2026 19:17

Copilot AI 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.

Pull request overview

This PR fixes locale loss during chat round dispatch by carrying the submitter’s AccessContext.Locale on the pending user message and ensuring the submitter identity snapshot ignores non-user principals (hub-shaped and system-security). This restores correct localization for round-emitted, user-facing messages regardless of scheduler hops.

Changes:

  • Capture the first real user submitter (excluding hub principals/System) and persist (ObjectId, Name, Locale) on pending user messages.
  • Rebuild the round’s AccessContext with the persisted locale (including the “ambient identity” branch) and enhance identity tracing to include the effective locale.
  • Strengthen ModelSubstitutionTest to submit under a German locale and assert German catalog output (plus a non-vacuity EN/DE sanity check), and add a What’s New entry.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/MeshWeaver.AI.Test/ModelSubstitutionTest.cs Makes the regression test non-vacuous by asserting German output and validating the locale rider.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-08-round-messages-in-your-language.md Documents the user-visible behavior change in What’s New.
src/MeshWeaver.AI/ThreadSubmission.cs Rebuilds AccessContext using SubmitterLocale and logs the effective locale during dispatch.
src/MeshWeaver.AI/ThreadInput.cs Extends user-message creation to include submitterLocale.
src/MeshWeaver.AI/Thread.cs Adds ThreadMessage.SubmitterLocale to persist the submitter’s language preference.
src/MeshWeaver.AI/HubThreadExtensions.cs Updates submitter capture to prefer real-user contexts and include locale; stamps locale on messages in submit paths.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/MeshWeaver.AI/ThreadSubmission.cs Outdated
Comment thread src/MeshWeaver.AI/Thread.cs
@github-actions

github-actions Bot commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

1 195 tests   1 195 ✅  5m 53s ⏱️
   11 suites      0 💤
   11 files        0 ❌

Results for commit 31ce2af.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

   11 files     11 suites   5m 10s ⏱️
1 875 tests 1 769 ✅ 106 💤 0 ❌
1 986 runs  1 880 ✅ 106 💤 0 ❌

Results for commit 31ce2af.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 684 tests   1 493 ✅  5m 56s ⏱️
   11 suites    191 💤
   11 files        0 ❌

Results for commit 31ce2af.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

685 tests   685 ✅  5m 28s ⏱️
 10 suites    0 💤
 10 files      0 ❌

Results for commit 31ce2af.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 172 tests   1 165 ✅  6m 7s ⏱️
   11 suites      7 💤
   11 files        0 ❌

Results for commit 31ce2af.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

1 836 tests   1 834 ✅  8m 20s ⏱️
   11 suites      2 💤
   11 files        0 ❌

Results for commit 31ce2af.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   65 files     65 suites   36m 57s ⏱️
8 447 tests 8 141 ✅ 306 💤 0 ❌
8 558 runs  8 252 ✅ 306 💤 0 ❌

Results for commit 31ce2af.

♻️ This comment has been updated with latest results.

AccessContext.Locale's contract is null/empty/unsupported → English, so an
empty string is a real "no preference" — but the identity trace printed it
with `?? "(default)"`, which only catches null. An empty Locale rendered as
a blank gap in the log line, which reads like the trace itself failed rather
than like the user has no preference — undermining the one diagnostic that
makes a "renders English for a German user" report readable from logs alone.

Same correction on SubmitterLocale's doc: it claimed null means no
preference, but the value is copied verbatim from AccessContext.Locale, so
empty reaches it unchanged and callers must use IsNullOrEmpty.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

A round's AccessContext is rebuilt without Locale — every user-facing round message renders in the default language

2 participants