Skip to content

Commit af0dd20

Browse files
committed
feat: NAMS Phase 10a -- SearchMessagesAsync + payload edge-case live tests
Adds INamsClient.SearchMessagesAsync (confirmed live: POST /v1/conversations/{id}/search), standalone -- deliberately not wired into the already-shipped RecallAsync pipeline. Adds 4 payload edge-case live tests, each verified against real live behavior before writing assertions: empty content (a genuine NAMS API inconsistency -- the bulk endpoint accepts what the single-message endpoint rejects), large messages (50K chars accepted at write time; round-trip tested at 5K to stay within our own recall character budget rather than measuring our own truncation logic), multi-message single turn, and code-like content with special characters.
1 parent fefedaa commit af0dd20

10 files changed

Lines changed: 239 additions & 0 deletions
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
# NAMS Phase 10a — SearchMessagesAsync + Payload Edge-Case Tests — Planning and Implementation Plan
2+
3+
**Prepared:** 2026-07-17
4+
**Branch:** `nams/phase10a-search-messages-and-payload-tests`
5+
**Purpose:** First increment of the follow-up Phase 10 work (10a-10d + a TCK Platinum research task), per the
6+
user's explicitly requested order: smallest/cleanest items first.
7+
8+
## 1. `INamsClient.SearchMessagesAsync`
9+
10+
Confirmed live: `POST /v1/conversations/{id}/search` with `{query, limit}`, returning
11+
`{messages: [...], searchType}` in exactly the existing `NamsMessage` shape (score/tokenCount present,
12+
conversationId absent -- matching the `recentMessages` context shape, not the bulk-add response shape).
13+
Phase 2 originally dropped this (`SearchMessagesAsync`) for lack of confirmation; added now, standalone.
14+
15+
**Deliberately NOT wired into `INamsRecallService.RecallAsync`** -- that's Phase 4-6's already-shipped, live-
16+
tested, unit-tested behavior. Changing what automatic recall does is a separate decision from adding a new
17+
client capability, and isn't part of this increment's scope. A future MCP `nams_search_messages` tool (a
18+
natural Phase 8 follow-up, per the plan's own `search_messages` span name already anticipated in Phase 9) is
19+
also explicitly deferred, not built here.
20+
21+
## 2. Payload edge-case live tests
22+
23+
Added to `NamsLiveConnectivityTests.cs`. Live-verified each edge case's REAL behavior before writing the
24+
test's assertions, rather than assuming what "should" happen:
25+
26+
- **Empty content** -- verified live that this is a genuine **NAMS API inconsistency**: the single-message
27+
endpoint (`POST /v1/conversations/{id}/messages`) rejects empty content with 400, but the bulk endpoint
28+
(`POST /v1/conversations/{id}/messages/bulk`) -- the *only* one `PersistTurnAsync` ever calls -- accepts it
29+
and returns a real message ID. The test asserts the actual (accepted) behavior, not the wrong assumption
30+
that empty content fails uniformly.
31+
- **Large message** -- verified live that NAMS's write path accepts at least 50,000 characters. For the
32+
round-trip half of the test, deliberately used 5,000 characters (comfortably under
33+
`NamsRecallOptions.MaxTotalCharacters`'s 8,000 default) after discovering the first version of this test
34+
failed for an unrelated reason: a single item larger than the entire recall character budget is silently
35+
excluded by `NamsRecallService`'s own `ApplyCharacterBudget` (correct, pre-existing, separately-tested
36+
behavior) -- the test was accidentally measuring our own truncation logic, not NAMS's payload handling.
37+
- **Multi-message single turn** -- 2 user + 2 assistant messages in one `PersistTurnAsync` call; asserts all
38+
4 come back with IDs from the one bulk request.
39+
- **Code-like content with special characters** (braces, quotes, escaped newlines, backticks) -- the plan's
40+
"non-text" payload category, interpreted as structured/code-like text (there's no binary/attachment
41+
concept anywhere in `NamsMessageToPersist`, so "non-text" can't mean literal binary).
42+
43+
## 3. Verification
44+
45+
- `dotnet build AgentMemory.slnx -c Release` -- 0 warnings, 0 errors.
46+
- `dotnet test tests/AgentMemory.Tests.Unit` -- full suite green, +1 new unit test (`SearchMessagesAsync`
47+
client test) plus 5 test-fake updates (every `INamsClient` implementer needed the new method).
48+
- `dotnet test tests/AgentMemory.Tests.Integration --filter "...NamsLiveConnectivityTests"` -- **11/11 live**
49+
(7 previous + 4 new), ~7s total, against the real NAMS SaaS.
50+
51+
## 4. Definition of done
52+
53+
- [x] `SearchMessagesAsync` added to `INamsClient`/`Neo4jNamsClientAdapter`, instrumented (Phase 9 metrics,
54+
operation name `search_messages` matching the plan's own span-name anticipation), unit tested.
55+
- [x] 4 payload edge-case live tests added, each verified against real live behavior first.
56+
- [x] Full unit + live suites green.
57+
- [ ] Self-reviewed, PR opened, CI green, merged to `main`.

src/AgentMemory.Nams/Client/INamsClient.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,15 @@ Task<IReadOnlyList<NamsEntity>> SearchEntitiesAsync(
4343
/// health probe.
4444
/// </summary>
4545
Task<IReadOnlyList<NamsEntity>> ListEntitiesAsync(int limit, CancellationToken cancellationToken);
46+
47+
/// <summary>
48+
/// Searches a conversation's own message history (<c>POST /v1/conversations/{id}/search</c>) --
49+
/// confirmed live as part of the Phase 10 scenario-matrix expansion. Phase 2's original design dropped
50+
/// this (as <c>SearchMessagesAsync</c>) for lack of confirmation at the time; it's added now, standalone,
51+
/// deliberately NOT wired into <c>INamsRecallService.RecallAsync</c>'s already-shipped, tested Phase 4-6
52+
/// behavior -- that would be a real behavior change to the automatic recall pipeline, a separate decision
53+
/// from adding the client capability itself.
54+
/// </summary>
55+
Task<IReadOnlyList<NamsMessage>> SearchMessagesAsync(
56+
string conversationId, string query, int limit, CancellationToken cancellationToken);
4657
}

src/AgentMemory.Nams/Client/Neo4jNamsClientAdapter.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,21 @@ public async Task<IReadOnlyList<NamsEntity>> ListEntitiesAsync(int limit, Cancel
9797
return response.Entities;
9898
}
9999

100+
public async Task<IReadOnlyList<NamsMessage>> SearchMessagesAsync(
101+
string conversationId, string query, int limit, CancellationToken cancellationToken)
102+
{
103+
// A POST verb, but a pure read-only query with no server-side side effects -- same idempotent-for-
104+
// retry-purposes treatment as SearchEntitiesAsync above.
105+
var path = $"conversations/{Uri.EscapeDataString(conversationId)}/search";
106+
var response = await InvokeAsync(
107+
"search_messages",
108+
() => BuildJsonRequest(HttpMethod.Post, path, new SearchMessagesRequestBody(query, limit)),
109+
isIdempotent: true,
110+
DeserializeAsync<SearchMessagesResponseBody>,
111+
cancellationToken).ConfigureAwait(false);
112+
return response.Messages;
113+
}
114+
100115
private async Task<T> InvokeAsync<T>(
101116
string operationName,
102117
Func<HttpRequestMessage> requestFactory,
@@ -184,4 +199,12 @@ private sealed record SearchEntitiesResponseBody(
184199

185200
private sealed record ListEntitiesResponseBody(
186201
[property: JsonPropertyName("entities")] IReadOnlyList<NamsEntity> Entities);
202+
203+
private sealed record SearchMessagesRequestBody(
204+
[property: JsonPropertyName("query")] string Query,
205+
[property: JsonPropertyName("limit")] int Limit);
206+
207+
private sealed record SearchMessagesResponseBody(
208+
[property: JsonPropertyName("messages")] IReadOnlyList<NamsMessage> Messages,
209+
[property: JsonPropertyName("searchType")] string? SearchType);
187210
}

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

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,115 @@ await persistence.PersistTurnAsync(
218218
found.Should().BeTrue("Unicode/emoji content should round-trip through NAMS byte-for-byte in the recent-messages tier");
219219
}
220220

221+
// ── Phase 10: payload edge cases (empty / max size / multi-message / non-text-like) ─────────────
222+
223+
[LiveNamsFact]
224+
public async Task PersistTurnAsync_EmptyContent_BulkEndpointAcceptsIt()
225+
{
226+
var services = _fixture.Services!;
227+
var resolver = services.GetRequiredService<INamsConversationResolver>();
228+
var persistence = services.GetRequiredService<INamsPersistenceService>();
229+
var conversation = await resolver.ResolveAsync(UniqueIdentity(), CancellationToken.None);
230+
231+
// Genuine NAMS API inconsistency, confirmed live: the single-message endpoint
232+
// (POST /v1/conversations/{id}/messages) rejects empty content with 400, but the bulk endpoint
233+
// (POST /v1/conversations/{id}/messages/bulk) -- the ONLY one PersistTurnAsync ever calls --
234+
// accepts it and returns a real message ID. Documenting actual behavior, not the (wrong) assumption
235+
// that empty content fails uniformly across both NAMS endpoints.
236+
var result = await persistence.PersistTurnAsync(
237+
conversation.NamsConversationId, userMessages: [new NamsMessageToPersist("")], assistantMessages: [], CancellationToken.None);
238+
239+
result.Outcome.Should().Be(NamsPersistenceOutcome.Persisted);
240+
result.PersistedMessageIds.Should().ContainSingle();
241+
}
242+
243+
[LiveNamsFact]
244+
public async Task PersistTurnAsync_LargeMessage_PersistsSuccessfully_AndEventuallyRoundTripsWithinTheDefaultRecallBudget()
245+
{
246+
var services = _fixture.Services!;
247+
var resolver = services.GetRequiredService<INamsConversationResolver>();
248+
var persistence = services.GetRequiredService<INamsPersistenceService>();
249+
var recall = services.GetRequiredService<INamsRecallService>();
250+
var conversation = await resolver.ResolveAsync(UniqueIdentity(), CancellationToken.None);
251+
252+
// Confirmed live: NAMS's write path accepts at least 50,000 characters in one message (separately
253+
// verified, not asserted here to keep this test's own runtime reasonable). For the round-trip half
254+
// of this test, deliberately use a size comfortably under NamsRecallOptions.MaxTotalCharacters'
255+
// default (8,000) -- a single item larger than the whole budget is silently excluded by
256+
// NamsRecallService's own ApplyCharacterBudget (correct, existing, separately-tested behavior), which
257+
// would make this test measure our own truncation logic instead of NAMS's payload handling.
258+
var marker = $"large-payload-{Guid.NewGuid():N} ";
259+
var largeContent = marker + new string('x', 5_000);
260+
var persistResult = await persistence.PersistTurnAsync(
261+
conversation.NamsConversationId, userMessages: [new NamsMessageToPersist(largeContent)], assistantMessages: [], CancellationToken.None);
262+
263+
persistResult.Outcome.Should().Be(NamsPersistenceOutcome.Persisted);
264+
265+
var found = await PollUntilAsync(
266+
async () =>
267+
{
268+
var recalled = await recall.RecallAsync(conversation.NamsConversationId, marker, CancellationToken.None);
269+
return recalled.Items.Any(i =>
270+
i.Category == NamsRecallCategory.RecentMessage && i.Content.Contains(marker, StringComparison.Ordinal));
271+
},
272+
timeout: TimeSpan.FromSeconds(30));
273+
274+
found.Should().BeTrue("a 5,000-character message (within the default recall budget) should round-trip through NAMS");
275+
}
276+
277+
[LiveNamsFact]
278+
public async Task PersistTurnAsync_MultipleMessagesInOneTurn_PersistsAllInOrder()
279+
{
280+
var services = _fixture.Services!;
281+
var resolver = services.GetRequiredService<INamsConversationResolver>();
282+
var persistence = services.GetRequiredService<INamsPersistenceService>();
283+
var conversation = await resolver.ResolveAsync(UniqueIdentity(), CancellationToken.None);
284+
285+
var id = Guid.NewGuid().ToString("N");
286+
var result = await persistence.PersistTurnAsync(
287+
conversation.NamsConversationId,
288+
userMessages: [new NamsMessageToPersist($"user-1-{id}"), new NamsMessageToPersist($"user-2-{id}")],
289+
assistantMessages: [new NamsMessageToPersist($"assistant-1-{id}"), new NamsMessageToPersist($"assistant-2-{id}")],
290+
CancellationToken.None);
291+
292+
result.Outcome.Should().Be(NamsPersistenceOutcome.Persisted);
293+
result.PersistedMessageIds.Should().HaveCount(4, "all 4 messages in the turn should be persisted in one bulk call");
294+
}
295+
296+
[LiveNamsFact]
297+
public async Task PersistTurnAsync_CodeLikeContentWithSpecialCharacters_EventuallyRoundTrips()
298+
{
299+
var services = _fixture.Services!;
300+
var resolver = services.GetRequiredService<INamsConversationResolver>();
301+
var persistence = services.GetRequiredService<INamsPersistenceService>();
302+
var recall = services.GetRequiredService<INamsRecallService>();
303+
var conversation = await resolver.ResolveAsync(UniqueIdentity(), CancellationToken.None);
304+
305+
// "Non-text" per the plan's payload matrix, interpreted as structured/code-like text content --
306+
// NamsMessageToPersist.Content is a string; NAMS has no binary/attachment concept to test against.
307+
var marker = Guid.NewGuid().ToString("N");
308+
var codeContent = $$"""
309+
marker: {{marker}}
310+
```csharp
311+
var x = new Dictionary<string, string> { ["a"] = "b\n\tc" };
312+
Console.WriteLine($"{x["a"]}");
313+
```
314+
""";
315+
await persistence.PersistTurnAsync(
316+
conversation.NamsConversationId, userMessages: [new NamsMessageToPersist(codeContent)], assistantMessages: [], CancellationToken.None);
317+
318+
var found = await PollUntilAsync(
319+
async () =>
320+
{
321+
var recalled = await recall.RecallAsync(conversation.NamsConversationId, marker, CancellationToken.None);
322+
return recalled.Items.Any(i =>
323+
i.Category == NamsRecallCategory.RecentMessage && i.Content.Contains(marker, StringComparison.Ordinal));
324+
},
325+
timeout: TimeSpan.FromSeconds(30));
326+
327+
found.Should().BeTrue("code-like content with braces/quotes/newlines should round-trip through NAMS");
328+
}
329+
221330
private static NamsConversationIdentity UniqueIdentity([CallerMemberName] string testName = "") => new()
222331
{
223332
ApplicationId = "agent-memory-dotnet-live-tests",

tests/AgentMemory.Tests.Unit/Nams/NamsConversationResolverTests.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ public Task<IReadOnlyList<NamsEntity>> SearchEntitiesAsync(
4444
string query, string? type, int limit, CancellationToken cancellationToken) =>
4545
throw new NotSupportedException();
4646

47+
public Task<IReadOnlyList<NamsMessage>> SearchMessagesAsync(
48+
string conversationId, string query, int limit, CancellationToken cancellationToken) =>
49+
throw new NotSupportedException();
50+
4751
public Task<IReadOnlyList<NamsEntity>> ListEntitiesAsync(int limit, CancellationToken cancellationToken) =>
4852
throw new NotSupportedException();
4953
}

tests/AgentMemory.Tests.Unit/Nams/NamsHealthCheckTests.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ public Task<IReadOnlyList<NamsEntity>> ListEntitiesAsync(int limit, Cancellation
3333
cancellationToken.ThrowIfCancellationRequested();
3434
return (OnListEntities ?? ((_, _) => Task.FromResult<IReadOnlyList<NamsEntity>>([])))(limit, cancellationToken);
3535
}
36+
37+
public Task<IReadOnlyList<NamsMessage>> SearchMessagesAsync(
38+
string conversationId, string query, int limit, CancellationToken cancellationToken) =>
39+
throw new NotSupportedException();
3640
}
3741

3842
private static NamsOptions ValidOptions() => new() { Endpoint = new Uri("https://nams.test/v1/"), ApiKey = "nams_key" };

tests/AgentMemory.Tests.Unit/Nams/NamsObservabilityTests.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,10 @@ public Task<IReadOnlyList<NamsEntity>> SearchEntitiesAsync(
256256
string query, string? type, int limit, CancellationToken cancellationToken) =>
257257
Task.FromResult<IReadOnlyList<NamsEntity>>([]);
258258

259+
public Task<IReadOnlyList<NamsMessage>> SearchMessagesAsync(
260+
string conversationId, string query, int limit, CancellationToken cancellationToken) =>
261+
throw new NotSupportedException();
262+
259263
public Task<IReadOnlyList<NamsEntity>> ListEntitiesAsync(int limit, CancellationToken cancellationToken) =>
260264
throw new NotSupportedException();
261265
}

tests/AgentMemory.Tests.Unit/Nams/NamsPersistenceServiceTests.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ public Task<IReadOnlyList<NamsEntity>> SearchEntitiesAsync(
3636
string query, string? type, int limit, CancellationToken cancellationToken) =>
3737
throw new NotSupportedException();
3838

39+
public Task<IReadOnlyList<NamsMessage>> SearchMessagesAsync(
40+
string conversationId, string query, int limit, CancellationToken cancellationToken) =>
41+
throw new NotSupportedException();
42+
3943
public Task<IReadOnlyList<NamsEntity>> ListEntitiesAsync(int limit, CancellationToken cancellationToken) =>
4044
throw new NotSupportedException();
4145
}

tests/AgentMemory.Tests.Unit/Nams/NamsRecallServiceTests.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ public Task<IReadOnlyList<NamsEntity>> SearchEntitiesAsync(
3838
return (OnSearchEntities ?? ((_, _, _) => Task.FromResult<IReadOnlyList<NamsEntity>>([])))(query, limit, cancellationToken);
3939
}
4040

41+
public Task<IReadOnlyList<NamsMessage>> SearchMessagesAsync(
42+
string conversationId, string query, int limit, CancellationToken cancellationToken) =>
43+
throw new NotSupportedException();
44+
4145
public Task<IReadOnlyList<NamsEntity>> ListEntitiesAsync(int limit, CancellationToken cancellationToken) =>
4246
throw new NotSupportedException();
4347
}

tests/AgentMemory.Tests.Unit/Nams/Neo4jNamsClientAdapterTests.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,25 @@ public async Task SearchEntitiesAsync_Success_UsesRetryDespitePostVerb()
135135
fake.Requests.Should().HaveCount(2); // search is a read despite the POST verb -- retries like other reads
136136
}
137137

138+
[Fact]
139+
public async Task SearchMessagesAsync_Success_UsesRetryDespitePostVerb()
140+
{
141+
var fake = new FakeHttpMessageHandler(
142+
() => new HttpResponseMessage(HttpStatusCode.ServiceUnavailable),
143+
() => Json(HttpStatusCode.OK, """
144+
{"messages":[{"id":"m1","content":"the quick brown fox","role":"user","score":0.8,"tokenCount":4}],"searchType":"vector"}
145+
"""));
146+
var adapter = CreateAdapter(fake);
147+
148+
var messages = await adapter.SearchMessagesAsync("conv-1", "quick brown fox", 5, CancellationToken.None);
149+
150+
messages.Single().Content.Should().Be("the quick brown fox");
151+
messages.Single().Score.Should().Be(0.8);
152+
fake.Requests.Should().HaveCount(2); // search is a read despite the POST verb -- retries like other reads
153+
var requestBody = await fake.Requests[1].Content!.ReadAsStringAsync();
154+
requestBody.Should().Contain("\"query\":\"quick brown fox\"").And.Contain("\"limit\":5");
155+
}
156+
138157
[Fact]
139158
public async Task ListEntitiesAsync_Success_DeserializesEntities_NoQueryInRequest()
140159
{

0 commit comments

Comments
 (0)