fix: self-review findings from Phase 10a branch (test naming, stale doc comment)

- Renamed the multi-message test to stop claiming order verification it can't actually
  prove (PersistedMessageIds has no content/role correlation); documented what it does verify.
- Fixed INamsClient's top-of-interface doc comment, stale since Phase 9 and now doubly wrong:
  it still said "4 operations" and called SearchMessagesAsync "dropped" even though this
  branch is exactly the trigger that comment named for adding it.

Re-verified: 11/11 live tests still green, full unit suite green.

Author: joslat

docs/reviews/NAMS_Phase10a_SearchMessagesAndPayloadTests_PlanningAndImplementationPlan.md

@@ -40,18 +40,36 @@ test's assertions, rather than assuming what "should" happen:
   "non-text" payload category, interpreted as structured/code-like text (there's no binary/attachment
   concept anywhere in `NamsMessageToPersist`, so "non-text" can't mean literal binary).
 
-## 3. Verification
+## 3. Self-review findings and fixes
+
+2 parallel reviewers (correctness/test-soundness + combined cross-file/conventions):
+
+- **Test over-claiming its name** (correctness angle): `PersistTurnAsync_MultipleMessagesInOneTurn_PersistsAllInOrder`
+  asserted only a bare count (4 IDs), never actually verifying order -- `NamsPersistenceResult.PersistedMessageIds`
+  has no content/role correlation, and NAMS's own bulk-response ordering guarantee (if any) isn't independently
+  confirmed, so nothing in this test could prove ordering even if it tried. Renamed to
+  `...PersistsAllFour` and added a doc comment stating exactly what is and isn't proven, rather than building a
+  shakier ordering-verification mechanism to match an overclaimed name.
+- **Stale interface doc comment** (cross-file angle): `INamsClient.cs`'s top-of-interface comment still said
+  "covers only the 4 operations" and called `SearchMessagesAsync` "dropped" -- both wrong after this branch
+  (and already wrong before it, post-Phase-9's `ListEntitiesAsync`). This branch is exactly the "add it once
+  verified live" trigger that comment itself named, so fixed it here rather than leaving it stale again.
+
+Re-verified after fixes: 11/11 live tests still green.
+
+## 4. Verification
 
 - `dotnet build AgentMemory.slnx -c Release` -- 0 warnings, 0 errors.
 - `dotnet test tests/AgentMemory.Tests.Unit` -- full suite green, +1 new unit test (`SearchMessagesAsync`
   client test) plus 5 test-fake updates (every `INamsClient` implementer needed the new method).
 - `dotnet test tests/AgentMemory.Tests.Integration --filter "...NamsLiveConnectivityTests"` -- **11/11 live**
   (7 previous + 4 new), ~7s total, against the real NAMS SaaS.
 
-## 4. Definition of done
+## 5. Definition of done
 
 - [x] `SearchMessagesAsync` added to `INamsClient`/`Neo4jNamsClientAdapter`, instrumented (Phase 9 metrics,
       operation name `search_messages` matching the plan's own span-name anticipation), unit tested.
 - [x] 4 payload edge-case live tests added, each verified against real live behavior first.
 - [x] Full unit + live suites green.
-- [ ] Self-reviewed, PR opened, CI green, merged to `main`.
+- [x] Self-reviewed and fixes applied.
+- [ ] PR opened, CI green, merged to `main`.

src/AgentMemory.Nams/Client/INamsClient.cs

@@ -7,11 +7,11 @@ namespace AgentMemory.Nams.Client;
 /// NAMS SDK type directly -- everything crossing that boundary is contained inside <c>AgentMemory.Nams</c> behind
 /// this interface (engineering plan §7 Phase 2, "Rule").
 ///
-/// Deliberately covers only the 4 operations confirmed against the pinned OpenAPI snapshot
-/// (<c>docs/reviews/nams-openapi-snapshot-2026-07-17.json</c>). A 5th method the engineering plan's own example
-/// included -- <c>SearchMessagesAsync</c> -- is dropped: the plan's own text already flags it as unconfirmed (no
-/// distinct search/query REST operation is documented), and building against a guessed endpoint shape would be
-/// worse than not building it. Add it once verified against a live sandbox (<c>strategy/NAMS/Neo4j_Questions.md</c>).
+/// Started (Phase 2) with only the 4 operations confirmed against the pinned OpenAPI snapshot
+/// (<c>docs/reviews/nams-openapi-snapshot-2026-07-17.json</c>); a 5th method the plan's own example included --
+/// <c>SearchMessagesAsync</c> -- was deliberately dropped at the time as unconfirmed. Both that method and
+/// <see cref="ListEntitiesAsync"/> (Phase 9) have since been confirmed against the live NAMS SaaS and added --
+/// see each method's own doc comment for when/why.
 /// </summary>
 internal interface INamsClient
 {

tests/AgentMemory.Tests.Integration/Nams/NamsLiveConnectivityTests.cs

@@ -274,8 +274,14 @@ public async Task PersistTurnAsync_LargeMessage_PersistsSuccessfully_AndEventual
         found.Should().BeTrue("a 5,000-character message (within the default recall budget) should round-trip through NAMS");
     }
 
+    /// <summary>
+    /// Deliberately does NOT assert message ordering -- <see cref="NamsPersistenceResult.PersistedMessageIds"/>
+    /// is a bare <c>IReadOnlyList&lt;string&gt;</c> with no content/role correlation, and NAMS's own bulk-add
+    /// response ordering guarantee (if any) isn't independently confirmed, so this can only prove all 4
+    /// messages in one turn reach NAMS via the one bulk call, not that they arrive in submission order.
+    /// </summary>
     [LiveNamsFact]
-    public async Task PersistTurnAsync_MultipleMessagesInOneTurn_PersistsAllInOrder()
+    public async Task PersistTurnAsync_MultipleMessagesInOneTurn_PersistsAllFour()
     {
         var services = _fixture.Services!;
         var resolver = services.GetRequiredService<INamsConversationResolver>();