x86 grader: 64-bit register expected values lose precision over the wire #116

New Issue

2026-05-11T19:23:38Z

spikerj commented

2026-05-11 19:23:38 +00:00

Status: Closed (fix shipped — opening here for the audit trail)
Reported by: Lesson 50103 failure ("Register rbx = 0x1122334455667800" displayed as expected, despite the lesson source asserting 0x11223344556677FF).
Affected component: RegisterStateAssertion round-trip between SpikerSoft.Business.Domain.Lessons.Curriculum.X86.X86LessonPlan and X86LessonGraderService on the SPA.

Symptom

On lessonId=50103 (bl, bh, bx: writing partial registers):

The lesson instructions, hint 3, and X86ReferenceSolutions.cs all agree that mov rbx, 0x1122334455667700 then mov bl, 0xFF should produce rbx = 0x11223344556677FF and pass.
The student types exactly that, the Blink WASM emulator correctly reports rbx = 0x11223344556677FF, but the grader fails with:

Register rbx = 0x1122334455667800
Got 0x11223344556677ff (1234605616436508671).

The "expected" the grader displays is the lesson's true expected value plus 1.

Root cause

RegisterStateAssertion.Expected is IReadOnlyDictionary<string, long>. The plan is JSON-serialized in X86LessonStrategyBase.BuildAttempt, stuffed into LessonAttemptState.TestCode, and shipped to the SPA as part of BrowserGradingState. The TS grader's mirror interface declared the field as Record<string, number>:

export interface RegisterStateAssertion {
    readonly $type: "registerState";
    readonly expected: Readonly<Record<string, number>>;
}

System.Text.Json writes a C# long as a bare JSON number. The browser's JSON.parse produces a JavaScript number — an IEEE 754 double with only 53 bits of mantissa. Around 2^60, the gap between consecutive representable doubles is 2^(60-52) = 256, so:

0x11223344556677FF = 1234605616436508671
Nearest representable doubles: 1234605616436508416 (0x1122334455667700, distance 255) and 1234605616436508672 (0x1122334455667800, distance 1)
Rounds up → the parsed number is 1234605616436508672 = 0x1122334455667800

BigInt(rawExpected) then converts the already-rounded double to a BigInt — fidelity is gone before BigInt ever sees it. The actual register snapshot is sent as a hex string and BigInt-parsed losslessly, so the comparison is 0x11223344556677FFn !== 0x1122334455667800n and the row fails. The doc-comment on RegisterStateAssertion ("the JSON serializer carries them as numbers, the grader reinterprets as BigInt on the SPA side for full 64-bit fidelity") was wishful thinking — BigInt cannot un-lose precision the JSON parse already discarded.

The server-side replay grader (BlinkX86LessonGradingExecutor) keeps everything in long/ulong and never round-trips through a JS number, which is why BlinkX86LessonGradingExecutorTests.cs passed for 0xFFFFFFFFFFFFFFFF. Only the SPA-side grading path was broken, and that's the one production uses for x86 lessons.

Affected lessons

Any register expected value whose magnitude exceeds 2^53 (0x20_0000_0000_0000) and whose low bits aren't conveniently zero. A scan of the x86 curriculum at the time of writing:

50103 — 0x11223344556677FFL — broken.
50202 — (rbx, -6L), (rcx, -5L) — fine (small magnitudes; the negative-fixup compensates).
All other Tiers 1-7 register assertions today — fine (small values).

So 50103 was the only currently-affected lesson, but the bug was latent for any future lesson author who writes a non-trivial 64-bit value.

Fix

Change the wire format for RegisterStateAssertion.Expected from JSON numbers to hex strings (mirroring what the actual register snapshot already does):

Backend — Add HexEncodedLongDictionaryConverter (a JsonConverter<IReadOnlyDictionary<string, long>>) that:
- Writes each value as "0x{value:X}" reinterpreted as ulong (so -6L becomes "0xFFFFFFFFFFFFFFFA").
- Reads strings (hex with optional 0x prefix) AND raw JSON numbers (back-compat for any older client / cached payload). Hex is parsed as ulong and reinterpreted to long.
Apply via [property: JsonConverter(typeof(HexEncodedLongDictionaryConverter))] on the record positional parameter.
Frontend — Change RegisterStateAssertion.expected type to Readonly<Record<string, string>>. BigInt(rawExpected) already accepts hex literals ("0x...") directly, so the grader gets a lossless bigint. The legacy "treat negative-as-unsigned" fix-up becomes a no-op (the wire is unsigned hex now), but stays as defensive code.
Tests — Add a regression test on each side for 0x11223344556677FFL to ensure the round-trip stays lossless.
Doc — Replace the wishful-thinking sentence on RegisterStateAssertion (X86LessonPlan.cs:80-88) with the actual contract.

Resolution

Closed. All four items implemented and validated.

Changes

File	Change
`spikersoft-backend/SpikerSoft.Business/Domain/Lessons/Curriculum/X86/X86LessonPlan.cs`	Added `HexEncodedLongDictionaryConverter` (write: unsigned hex with `0x` prefix; read: hex string OR raw JSON number for back-compat). Applied via `[property: JsonConverter(...)]` on `RegisterStateAssertion.Expected`. Replaced the misleading `<summary>` doc-comment with the actual contract.
`spikersoft-backend/SpikerSoft.Tests.Unit/Domain/CodeExecution/Execution/BlinkX86LessonGradingExecutorTests.cs`	Added three regressions: `RegisterStateAssertion_PreservesValuesAbove2to53Losslessly` (the bug-for-bug test using `0x11223344556677FF`), `RegisterStateAssertion_WireFormat_EmitsExpectedAsHexStrings` (asserts the on-the-wire JSON shape), `RegisterStateAssertion_ReadSide_TolerantOfLegacyJsonNumbers` (back-compat for any cached payload from the old serializer).
`spikersoft-angular/libraries/features/dev-tools-x86-playground/src/lib/services/x86-lesson-grader.service.ts`	`RegisterStateAssertion.expected` retyped from `Record<string, number>` → `Record<string, string>`. `gradeRegisters` parses with `BigInt(rawExpected)`; the unsigned-fixup branch becomes a defensive no-op since the new wire is always unsigned hex. Updated the JSDoc to document the contract.
`spikersoft-angular/libraries/features/dev-tools-x86-playground/src/lib/services/x86-lesson-grader.service.spec.ts`	Existing register tests migrated to the string form. Added `preserves values above 2^53 losslessly (regression for lesson 50103)` exercising `0x11223344556677ff` end-to-end.

Validation

dotnet test --filter "FullyQualifiedName~BlinkX86LessonGradingExecutorTests|FullyQualifiedName~X86CurriculumTests" — 332 passed, 0 failed. Every existing assertion still passes; the three new regressions also pass. The smoke test that round-trips every Challenge's TestCode through JsonSerializer.Deserialize<X86LessonPlan> continues to pass, confirming the converter doesn't break any existing lesson's serialization.
nx test feature-dev-tools-x86-playground — 20 passed, 0 failed, including the new regression case that ships expected: { rbx: "0x11223344556677ff" } and verifies the row passes byte-for-byte.
Dev server (npm run serve:spikersoft-development) rebuilt cleanly with no new warnings; the x86-playground lazy chunk size is unchanged at 419.18 kB.

Wire-format change summary

Before:

{ "$type": "registerState", "expected": { "rbx": 1234605616436508671 } }

After:

{ "$type": "registerState", "expected": { "rbx": "0x11223344556677FF" } }

The read side accepts both shapes, so any in-flight payload from the previous serializer (cached BrowserGradingState in the user's IndexedDB queue, Redis-cached LessonAttempt.TestCode) will still grade correctly after deploy. New writes always emit the hex-string form.

**Status:** Closed (fix shipped — opening here for the audit trail) **Reported by:** Lesson 50103 failure ("Register rbx = 0x1122334455667800" displayed as expected, despite the lesson source asserting `0x11223344556677FF`). **Affected component:** `RegisterStateAssertion` round-trip between `SpikerSoft.Business.Domain.Lessons.Curriculum.X86.X86LessonPlan` and `X86LessonGraderService` on the SPA. ## Symptom On `lessonId=50103` (`bl, bh, bx: writing partial registers`): - The lesson instructions, hint 3, and `X86ReferenceSolutions.cs` all agree that `mov rbx, 0x1122334455667700` then `mov bl, 0xFF` should produce `rbx = 0x11223344556677FF` and pass. - The student types exactly that, the Blink WASM emulator correctly reports `rbx = 0x11223344556677FF`, but the grader fails with: > Register rbx = 0x1122334455667800 > Got 0x11223344556677ff (1234605616436508671). The "expected" the grader displays is the lesson's true expected value plus 1. ## Root cause `RegisterStateAssertion.Expected` is `IReadOnlyDictionary<string, long>`. The plan is JSON-serialized in `X86LessonStrategyBase.BuildAttempt`, stuffed into `LessonAttemptState.TestCode`, and shipped to the SPA as part of `BrowserGradingState`. The TS grader's mirror interface declared the field as `Record<string, number>`: ```typescript export interface RegisterStateAssertion { readonly $type: "registerState"; readonly expected: Readonly<Record<string, number>>; } ``` `System.Text.Json` writes a C# `long` as a bare JSON number. The browser's `JSON.parse` produces a JavaScript `number` — an IEEE 754 double with only 53 bits of mantissa. Around 2^60, the gap between consecutive representable doubles is `2^(60-52) = 256`, so: - `0x11223344556677FF` = `1234605616436508671` - Nearest representable doubles: `1234605616436508416` (`0x1122334455667700`, distance 255) and `1234605616436508672` (`0x1122334455667800`, distance **1**) - Rounds **up** → the parsed `number` is `1234605616436508672` = `0x1122334455667800` `BigInt(rawExpected)` then converts the already-rounded double to a BigInt — fidelity is gone before BigInt ever sees it. The actual register snapshot is sent as a hex string and `BigInt`-parsed losslessly, so the comparison is `0x11223344556677FFn !== 0x1122334455667800n` and the row fails. The doc-comment on `RegisterStateAssertion` ("the JSON serializer carries them as numbers, the grader reinterprets as `BigInt` on the SPA side for full 64-bit fidelity") was wishful thinking — `BigInt` cannot un-lose precision the JSON parse already discarded. The server-side replay grader (`BlinkX86LessonGradingExecutor`) keeps everything in `long`/`ulong` and never round-trips through a JS number, which is why `BlinkX86LessonGradingExecutorTests.cs` passed for `0xFFFFFFFFFFFFFFFF`. Only the SPA-side grading path was broken, and that's the one production uses for x86 lessons. ## Affected lessons Any register expected value whose magnitude exceeds 2^53 (`0x20_0000_0000_0000`) and whose low bits aren't conveniently zero. A scan of the x86 curriculum at the time of writing: - 50103 — `0x11223344556677FFL` — **broken**. - 50202 — `(rbx, -6L), (rcx, -5L)` — fine (small magnitudes; the negative-fixup compensates). - All other Tiers 1-7 register assertions today — fine (small values). So 50103 was the only currently-affected lesson, but the bug was latent for any future lesson author who writes a non-trivial 64-bit value. ## Fix Change the wire format for `RegisterStateAssertion.Expected` from JSON numbers to hex strings (mirroring what the actual register snapshot already does): 1. **Backend** — Add `HexEncodedLongDictionaryConverter` (a `JsonConverter<IReadOnlyDictionary<string, long>>`) that: - Writes each value as `"0x{value:X}"` reinterpreted as `ulong` (so `-6L` becomes `"0xFFFFFFFFFFFFFFFA"`). - Reads strings (hex with optional `0x` prefix) AND raw JSON numbers (back-compat for any older client / cached payload). Hex is parsed as `ulong` and reinterpreted to `long`. Apply via `[property: JsonConverter(typeof(HexEncodedLongDictionaryConverter))]` on the record positional parameter. 2. **Frontend** — Change `RegisterStateAssertion.expected` type to `Readonly<Record<string, string>>`. `BigInt(rawExpected)` already accepts hex literals (`"0x..."`) directly, so the grader gets a lossless `bigint`. The legacy "treat negative-as-unsigned" fix-up becomes a no-op (the wire is unsigned hex now), but stays as defensive code. 3. **Tests** — Add a regression test on each side for `0x11223344556677FFL` to ensure the round-trip stays lossless. 4. **Doc** — Replace the wishful-thinking sentence on `RegisterStateAssertion` (`X86LessonPlan.cs:80-88`) with the actual contract. ## Resolution **Closed.** All four items implemented and validated. ### Changes | File | Change | |---|---| | `spikersoft-backend/SpikerSoft.Business/Domain/Lessons/Curriculum/X86/X86LessonPlan.cs` | Added `HexEncodedLongDictionaryConverter` (write: unsigned hex with `0x` prefix; read: hex string OR raw JSON number for back-compat). Applied via `[property: JsonConverter(...)]` on `RegisterStateAssertion.Expected`. Replaced the misleading `<summary>` doc-comment with the actual contract. | | `spikersoft-backend/SpikerSoft.Tests.Unit/Domain/CodeExecution/Execution/BlinkX86LessonGradingExecutorTests.cs` | Added three regressions: `RegisterStateAssertion_PreservesValuesAbove2to53Losslessly` (the bug-for-bug test using `0x11223344556677FF`), `RegisterStateAssertion_WireFormat_EmitsExpectedAsHexStrings` (asserts the on-the-wire JSON shape), `RegisterStateAssertion_ReadSide_TolerantOfLegacyJsonNumbers` (back-compat for any cached payload from the old serializer). | | `spikersoft-angular/libraries/features/dev-tools-x86-playground/src/lib/services/x86-lesson-grader.service.ts` | `RegisterStateAssertion.expected` retyped from `Record<string, number>` → `Record<string, string>`. `gradeRegisters` parses with `BigInt(rawExpected)`; the unsigned-fixup branch becomes a defensive no-op since the new wire is always unsigned hex. Updated the JSDoc to document the contract. | | `spikersoft-angular/libraries/features/dev-tools-x86-playground/src/lib/services/x86-lesson-grader.service.spec.ts` | Existing register tests migrated to the string form. Added `preserves values above 2^53 losslessly (regression for lesson 50103)` exercising `0x11223344556677ff` end-to-end. | ### Validation - `dotnet test --filter "FullyQualifiedName~BlinkX86LessonGradingExecutorTests|FullyQualifiedName~X86CurriculumTests"` — **332 passed, 0 failed.** Every existing assertion still passes; the three new regressions also pass. The smoke test that round-trips every Challenge's `TestCode` through `JsonSerializer.Deserialize<X86LessonPlan>` continues to pass, confirming the converter doesn't break any existing lesson's serialization. - `nx test feature-dev-tools-x86-playground` — **20 passed, 0 failed**, including the new regression case that ships `expected: { rbx: "0x11223344556677ff" }` and verifies the row passes byte-for-byte. - Dev server (`npm run serve:spikersoft-development`) rebuilt cleanly with no new warnings; the x86-playground lazy chunk size is unchanged at `419.18 kB`. ### Wire-format change summary Before: ```json { "$type": "registerState", "expected": { "rbx": 1234605616436508671 } } ``` After: ```json { "$type": "registerState", "expected": { "rbx": "0x11223344556677FF" } } ``` The read side accepts both shapes, so any in-flight payload from the previous serializer (cached `BrowserGradingState` in the user's IndexedDB queue, Redis-cached `LessonAttempt.TestCode`) will still grade correctly after deploy. New writes always emit the hex-string form.

spikerj closed this issue

2026-05-11 19:24:09 +00:00

spikerj referenced this issue

2026-07-04 20:44:38 +00:00

[Bug][E2E flake] /geography/:countryCode click-through — Leaflet "Cannot read properties of undefined (reading 'appendChild')" pageerror on CI only #363

spikerj referenced this issue

2026-07-06 04:01:03 +00:00

[Security][CRITICAL] SignalR hubs missing class-level [Authorize] — CodeExecutionHub / DashboardHub / ClusterHub open to anonymous connections #401

spikerj referenced this issue

2026-07-06 04:12:26 +00:00

[Security][CRITICAL] SignalR hubs missing class-level [Authorize] — CodeExecutionHub / DashboardHub / ClusterHub open to anonymous connections #401

spikerj referenced this issue