chore: address cleanup review follow-ups and Postgres test flake

Fix gig persistence integration flake with FakeTimeProvider on the Postgres
host, restore StringComparer.Ordinal on quest counter copies, remove the Bruno
docs block, and record resolved review items.

bruno/neon-sprawl-server/collection.bru

@@ -1,7 +1,3 @@
 meta {
   name: Neon Sprawl Server
 }
-
-docs {
-  chore/cleanup analyzer pass: no HTTP route or request-body changes.
-}

docs/reviews/2026-06-27-chore-cleanup.md

@@ -0,0 +1,57 @@
+# Code review — chore/cleanup analyzer pass
+
+**Date:** 2026-06-27  
+**Scope:** Branch `chore/cleanup` — commit `b233fa0` vs `2cf8bcd` (merge base on `main`)  
+**Base:** `2cf8bcd` (merge of PR #190)
+
+**Follow-up:** Suggestions and nits below are **done** (strikethrough + **Done.**).
+
+## Verdict
+
+**Approve with nits**
+
+## Summary
+
+This is a broad mechanical refactor across `NeonSprawl.Server` and `NeonSprawl.Server.Tests` (231 files, net −524 lines): collection expressions, primary-constructor catalog types, concrete private helper return types (CA1859), logging `IsEnabled` guards (CA1873), record XML doc fixes (RCS1263/RCS1139), `nameof` for SQL parameters, quest-store mutator delegate → `Func` + `MutateRowResult` record struct, removal of redundant `using` directives via new `GlobalUsings.cs`, and a substantial `csharp-style.md` appendix documenting the patterns. No HTTP routes, request bodies, or game contracts change. Build is clean (0 warnings). Full `dotnet test` passed **905/906** locally with one intermittent Postgres integration failure under parallel load; the same Postgres specs pass in isolation on this branch and on the parent commit.
+
+## Documentation checked
+
+| Path | Result |
+|------|--------|
+| `docs/plans/` (NEO-* / E7M4 stories) | **N/A** — unticketed `chore:` branch; no feature acceptance criteria |
+| `.cursor/rules/csharp-style.md` | **Matches** — new sections document patterns applied in this diff (collection expressions, CA1859/CA1873, RCS1015, record XML docs, test GlobalUsings) |
+| `docs/decomposition/modules/module_dependency_register.md` | **N/A** — no module behavior or authority changes |
+| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **N/A** — no implementation-status update required |
+| `bruno/neon-sprawl-server/collection.bru` | **Matches** — ~~Partially matches~~ docs block removed (**Done.**) |
+
+## Blocking issues
+
+None.
+
+## Suggestions
+
+1. ~~**Remove the Bruno `docs { }` block** in `bruno/neon-sprawl-server/collection.bru`. The note (“chore/cleanup analyzer pass: no HTTP route or request-body changes”) belongs in the PR/commit message, not in the Bruno collection metadata consumers may read as API documentation.~~ **Done.** Removed the `docs { }` block from `collection.bru`.
+
+2. ~~**Preserve explicit `StringComparer.Ordinal` in quest objective counter copies.** In `PostgresPlayerQuestStateStore.MutateObjectiveCounter`, the old code used `new Dictionary<string, int>(row.ObjectiveCounters, StringComparer.Ordinal)`. The new `ToDictionary` call relies on default string equality (ordinal in practice). For consistency with the rest of the quests module and defense-in-depth, pass `StringComparer.Ordinal` to `ToDictionary` or keep the dictionary copy constructor.~~ **Done.** `ToDictionary(..., StringComparer.Ordinal)` in `PostgresPlayerQuestStateStore.cs`.
+
+3. ~~**Confirm Postgres integration stability under parallel xUnit.** Full-suite runs intermittently fail one of `FactionStandingPersistenceIntegrationTests`, `GigProgressionGrantPersistenceIntegrationTests`, or `SkillProgressionGrantPersistenceIntegrationTests` (expected persisted value vs `0`), while filtered runs pass on both `2cf8bcd` and `b233fa0`. This looks like pre-existing collection/fixture contention rather than a logic regression from this diff, but worth a quick check that `[Collection("Postgres integration")]` isolation is sufficient before merge if CI runs the full suite in parallel.~~ **Done.** Root cause for gig flake: `GigProgressionGrantPersistenceIntegrationTests` used wall-clock `Task.Delay` while cooldown uses `TimeProvider`; casts could return HTTP 200 with `on_cooldown` (`Accepted: false`) without defeat. Fixed by registering `FakeTimeProvider` on `PostgresWebApplicationFactory` and advancing the fake clock in the gig persistence test (same pattern as `InMemoryWebApplicationFactory`). Full suite **906/906** after fix.
+
+## Nits
+
+- ~~Nit: `GlobalUsings.cs` pulls in domain namespaces (`NeonSprawl.Server.Game.Npc`, `NeonSprawl.Server.Tests.Game.Npc`, etc.). Works today (0 build warnings), but a one-line file comment listing intentional globals would help future contributors avoid accidental name clashes.~~ **Done.** File header comment added.
+- ~~Nit: `QuestProgressIds.cs` has a whitespace-only trailing blank line before the closing brace — harmless; could drop in a follow-up.~~ **Done.**
+- Nit: `QuestStateOperations.DenyAccept` no longer threads `playerId` / `questId`; telemetry comment correctly says “capture at call site when wiring E9.M1.” Fine for now — just remember at E9.M1.
+- Nit: `NpcRuntimeOperations.AdvanceAll` drops the post–idle→aggro `TryGet` refresh; the while-loop `TryGet` on the first iteration makes this equivalent. No action needed.
+- Nit: `PostgresPositionStateStore` drops unused `IOptions<GamePositionOptions>` / startup validation — correct dead-code removal; DI registration unchanged.
+
+## Verification
+
+```bash
+cd server/NeonSprawl.Server && dotnet build
+cd server/NeonSprawl.Server.Tests && dotnet test
+cd server/NeonSprawl.Server.Tests && dotnet test --filter "FullyQualifiedName~PersistenceIntegrationTests"
+```
+
+Expect **906** passed; if a single Postgres persistence test fails under full parallel run, re-run filtered as above to distinguish flake from regression.
+
+Manual Godot QA: none (no client or player-visible surface changes).

server/NeonSprawl.Server.Tests/Game/Gigs/GigProgressionGrantPersistenceIntegrationTests.cs

@@ -24,7 +24,10 @@ public sealed class GigProgressionGrantPersistenceIntegrationTests(PostgresInteg
             TargetId = PrototypeNpcRegistry.PrototypeNpcMeleeId,
         };
 
-        // Act — defeat alpha through first host (four pulses)
+        // Act — defeat alpha through first host (pulse loop; fake clock avoids wall-clock cooldown flakes)
+        Assert.NotNull(Factory.FakeClock);
+        var clock = Factory.FakeClock;
+        clock.Advance(TimeSpan.FromSeconds(3) + TimeSpan.FromMilliseconds(50));
         using (var firstClient = Factory.CreateClient())
         {
             await firstClient.PostAsJsonAsync(
@@ -41,15 +44,26 @@ public sealed class GigProgressionGrantPersistenceIntegrationTests(PostgresInteg
                     SchemaVersion = TargetSelectRequest.CurrentSchemaVersion,
                     TargetId = PrototypeNpcRegistry.PrototypeNpcMeleeId,
                 });
-            for (var i = 0; i < 4; i++)
+            AbilityCastResponse? defeatBody = null;
+            for (var i = 0; i < 12; i++)
             {
                 var response = await firstClient.PostAsJsonAsync("/game/players/dev-local-1/ability-cast", cast);
                 response.EnsureSuccessStatusCode();
-                if (i < 3)
+                var body = await response.Content.ReadFromJsonAsync<AbilityCastResponse>();
+                Assert.NotNull(body);
+                Assert.True(body!.Accepted, body.ReasonCode);
+                Assert.NotNull(body.CombatResolution);
+                if (body.CombatResolution!.TargetDefeated)
                 {
-                    await Task.Delay(TimeSpan.FromSeconds(3.2));
+                    defeatBody = body;
+                    break;
                 }
+
+                clock.Advance(TimeSpan.FromSeconds(3) + TimeSpan.FromMilliseconds(50));
             }
+
+            Assert.NotNull(defeatBody);
+            Assert.True(defeatBody!.CombatResolution!.TargetDefeated);
         }
 
         await using var secondFactory = new PostgresWebApplicationFactory();

server/NeonSprawl.Server.Tests/Game/PositionState/PostgresWebApplicationFactory.cs

@@ -1,6 +1,8 @@
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Mvc.Testing;
+using Microsoft.AspNetCore.TestHost;
 using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.Time.Testing;
 using NeonSprawl.Server.Game.Combat;
 using NeonSprawl.Server.Game.Crafting;
 using NeonSprawl.Server.Game.Gathering;
@@ -12,6 +14,9 @@ namespace NeonSprawl.Server.Tests.Game.PositionState;
 /// <summary>Integration host with <see cref="PositionStateServiceCollectionExtensions.NeonSprawlConnectionStringName"/> bound from environment.</summary>
 public sealed class PostgresWebApplicationFactory : WebApplicationFactory<Program>
 {
+    /// <summary>Controllable UTC clock for cast cooldown integration tests (NEO-32).</summary>
+    public FakeTimeProvider? FakeClock { get; private set; }
+
     protected override void ConfigureWebHost(IWebHostBuilder builder)
     {
         var cs = Environment.GetEnvironmentVariable("ConnectionStrings__NeonSprawl");
@@ -60,5 +65,20 @@ public sealed class PostgresWebApplicationFactory : WebApplicationFactory<Progra
                 ["ConnectionStrings:NeonSprawl"] = cs.Trim(),
             });
         });
+
+        builder.ConfigureTestServices(services =>
+        {
+            for (var i = services.Count - 1; i >= 0; i--)
+            {
+                if (services[i].ServiceType == typeof(TimeProvider))
+                {
+                    services.RemoveAt(i);
+                }
+            }
+
+            var fakeTime = new FakeTimeProvider(new DateTimeOffset(2026, 4, 30, 12, 0, 0, TimeSpan.Zero));
+            FakeClock = fakeTime;
+            services.AddSingleton<TimeProvider>(fakeTime);
+        });
     }
 }

server/NeonSprawl.Server.Tests/GlobalUsings.cs

@@ -1,3 +1,4 @@
+// Intentional test-project globals (avoid duplicating per-file usings; watch for name clashes with local types).
 global using Microsoft.Extensions.DependencyInjection;
 global using Microsoft.Extensions.Logging.Abstractions;
 global using Microsoft.Extensions.Options;

server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestStateStore.cs

@@ -200,7 +200,8 @@ public sealed class PostgresPlayerQuestStateStore(Npgsql.NpgsqlDataSource dataSo
 
             Dictionary<string, int> counters = row.ObjectiveCounters.ToDictionary(
                 static pair => pair.Key,
-                static pair => pair.Value);
+                static pair => pair.Value,
+                StringComparer.Ordinal);
             counters[objective] = newCount;
             return new MutateRowResult(
                 true,

server/NeonSprawl.Server/Game/Quests/QuestProgressIds.cs

@@ -33,5 +33,4 @@ public static class QuestProgressIds
 
         return $"{p}\0{q}";
     }
-
 }