Measure the ambient-culture ban instead of only asserting it (#3203) - #3351
Conversation
Four documents state that resolution is explicit off the viewer and never off CultureInfo.CurrentUICulture. Nothing measured it. That is the shape AGENTS.md warns about — prose asserting a guard that does not exist — and #3203 is what it costs: the fleet accumulated culture-less date formatting for months with every check green. 🚨 The shape that actually ships is the IMPLICIT one. Nobody writes CurrentUICulture on purpose; they write `timestamp.Humanize()`, which reaches for it silently. EVERY instance found across the fleet during #3203 was that form, not a named symbol — so this bans the CALL SHAPE as well as the symbol: · CultureInfo.Current(UI)Culture / CurrentThread.Current(UI)Culture as EXPRESSIONS · `.Humanize()` with an empty argument list (Humanize(culture: …) is fine) Comments and string literals are masked first, because several files in src/ explain this ban in their remarks and naming a defect is the opposite of committing it — all 8 CurrentUICulture matches in src/ today are prose. Why it matters, concretely: on Blazor Server the ambient culture is the CONTAINER's, one value shared by every viewer being served at that moment. A timestamp formatted through it comes out in whichever language the pod is set to, so the same message reads "vor 2 Stunden" to an English reader and "2 hours ago" to a German one depending on which replica answered — worse than untranslated, because it cannot be reproduced from the request. 🚨 A RATCHET AT ZERO, seeded with nothing: src/ has no offender today, so there is no allow file and no debt to grandfather. The fix for a red is never a new entry; it is to state the culture off AccessContext.Locale / host.ViewerLocale(), or the invariant one where the value sits inside deliberately unlocalized text. Evidence floor, because a guard that scans nothing passes: the scanned root must exist and the scan must read more than 800 files (src/ held 1258 .cs files when the floor was set). Verified it DISCRIMINATES rather than merely passing: tree as-is 1/1 pass offender injected 1/1 FAIL, naming both shapes with file:line Localization.md now names the guard, so the rule and its measurement travel together. Pairs-with: none — a test and a doc page; removes no public surface. Refs #3203 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new guard and the updated documentation both imply coverage of additional implicit-culture shapes (e.g., ToString("N2")) that the test does not currently measure, which creates a false sense of enforcement.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds a zero-tolerance test guard to ensure src/ does not resolve culture implicitly (via ambient culture) and updates the localization architecture documentation to state that this rule is now enforced by tests, addressing the “prose-only guard” gap highlighted in #3203.
Changes:
- Add
AmbientCultureRatchetGuardto scansrc/for ambient-culture symbol usage and culture-less.Humanize(). - Update
Localization.mdto document the new measured/ratcheted enforcement.
File summaries
| File | Description |
|---|---|
| test/MeshWeaver.Documentation.Test/AmbientCultureRatchetGuard.cs | New ratchet guard test that scans src/ for ambient-culture resolution patterns and fails with file/line evidence. |
| src/MeshWeaver.Documentation/Data/Architecture/Localization.md | Docs update explaining the viewer-locale rule and referencing the new guard as measurement. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| 🚨 **The rule is now MEASURED, and the shape it measures is the implicit one.** Nobody writes | ||
| `CurrentUICulture` on purpose; they write `timestamp.Humanize()` or `value.ToString("N2")`, which | ||
| reach for it silently — every instance found across the fleet during | ||
| [#3203](https://github.com/Systemorph/MeshWeaver/issues/3203) was a culture-less `Humanize()`, not a | ||
| named symbol. `AmbientCultureRatchetGuard` (`test/MeshWeaver.Documentation.Test`) therefore bans the | ||
| CALL SHAPE as well as the symbol, over all of `src/`, with comments and string literals masked so | ||
| that explaining the ban is not mistaken for committing it. |
| /// <para><b>The shape that actually ships is the IMPLICIT one.</b> Nobody writes | ||
| /// <c>CurrentUICulture</c> on purpose; they write <c>timestamp.Humanize()</c> and | ||
| /// <c>value.ToString("N2")</c>, which reach for it silently. Every instance found in the fleet | ||
| /// during Systemorph/MeshWeaver#3203 was of that form — a culture-less <c>Humanize()</c> — which is | ||
| /// why this guard bans the CALL SHAPE rather than the symbol.</para> |
Test Results (shard 0)251 tests 251 ✅ 2m 17s ⏱️ Results for commit bace70f. |
Test Results (shard 3)369 tests 367 ✅ 47s ⏱️ Results for commit bace70f. |
Test Results (shard 1)475 tests 475 ✅ 37s ⏱️ Results for commit bace70f. |
Test Results (shard 5) 4 files 4 suites 1m 42s ⏱️ Results for commit bace70f. |
Test Results (shard 4)2 316 tests 2 316 ✅ 2m 40s ⏱️ Results for commit bace70f. |
Test Results (shard 2)588 tests 396 ✅ 2m 56s ⏱️ Results for commit bace70f. |
Test Results 15 files 15 suites 11m 1s ⏱️ Results for commit bace70f. |
Four documents state that resolution is explicit off the viewer and never off
CultureInfo.CurrentUICulture. Nothing measured it. That is the shape AGENTS.md warns about — proseasserting a guard that does not exist — and #3203 is what it cost: the fleet accumulated culture-less
date formatting for months with every check green.
The shape that actually ships is the implicit one
Nobody writes
CurrentUICultureon purpose; they writetimestamp.Humanize(), which reaches for itsilently. Every instance found across the fleet during #3203 was that form, not a named symbol.
So the guard bans the call shape as well as the symbol:
CultureInfo.Current(UI)Culture/CurrentThread.Current(UI)Cultureas expressions;.Humanize()with an empty argument list —Humanize(culture: …)is fine.Comments and string literals are masked first: several files in
src/explain this ban in theirremarks, and naming a defect is the opposite of committing it. All 8
CurrentUICulturematches insrc/today are prose.Why it matters concretely
On Blazor Server the ambient culture is the container's — one value shared by every viewer being
served at that moment. A timestamp formatted through it comes out in whichever language the pod is
set to, so the same message reads "vor 2 Stunden" to an English reader and "2 hours ago" to a
German one depending on which replica answered. That is worse than untranslated: it cannot be
reproduced from the request.
A ratchet at zero
src/has no offender today, so there is no allow file and no debt to grandfather. The fix for ared is never a new entry — it is to state the culture off
AccessContext.Locale/host.ViewerLocale(), orCultureInfo.InvariantCulturewhere the value sits inside deliberatelyunlocalized text.
Evidence floor, because a guard that scans nothing passes: the scanned root must exist and the
scan must read more than 800 files (
src/held 1258.csfiles when the floor was set).Verification — it discriminates, it does not merely pass
d.Humanize()+CultureInfo.CurrentUICulture)file:lineDocumentationLinkIntegrityTestalso passes.Localization.mdnow names the guard, so the rule andits measurement travel together.
Pairs-with: none — a test and a doc page; removes no public surface.
Refs #3203