Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/AgentMemory.Abstractions/Options/MemoryOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,32 @@ public sealed record MemoryOptions
/// </remarks>
public bool OmitEmbeddingsFromRecall { get; init; }

/// <summary>
/// Skips the escalation ladder for an owner that holds no rows of the searched label (2.13).
/// </summary>
/// <remarks>
/// <para>
/// An empty owner-scoped vector search escalates: a widened probe over the <b>global</b> index,
/// then an owner-scoped scan. For an owner holding nothing of that label both are futile by
/// construction, and the widened probe is the costly one — it asks the index for up to 2,000
/// candidates across the whole corpus to find rows that do not exist.
/// </para>
/// <para>
/// <b>Measured before enabling, because shortening the ladder is a trade rather than a free win.</b>
/// A <i>starved</i> owner — many rows, crowded out of the global top-K by noisier neighbours — is
/// genuinely recovered by escalation, so a blanket removal loses real answers. The large-owner arm
/// showed the two populations are cleanly separable: the empty owner returns nothing at every
/// rung, while the starved owner's rows are found. This option skips the ladder only for the first.
/// </para>
/// <para>
/// Off by default. The results are identical either way — an owner with nothing to find finds
/// nothing — so this is purely a cost saving; but it is gated because an existence probe that
/// disagreed with the search's own scoping would skip a rescue that would have worked, and a
/// silent recall loss is not worth one avoided query.
/// </para>
/// </remarks>
public bool SkipEscalationWhenOwnerHasNoRows { get; init; }

// NOTE: extraction at the Core layer is explicit (call ExtractAndPersistAsync /
// ExtractFromSessionAsync). Automatic extraction on message persist is an adapter concern, configured
// by AgentFrameworkOptions.AutoExtractOnPersist. The former EnableAutoExtraction flag here was read
Expand Down
54 changes: 54 additions & 0 deletions src/AgentMemory.Neo4j/Queries/OwnerRowExistence.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
namespace AgentMemory.Neo4j.Queries;

/// <summary>
/// "Does this owner hold any rows of this label at all?" — the check that separates a starved owner
/// from an empty one before the escalation ladder is climbed (PLAN 2.13).
/// </summary>
/// <remarks>
/// <para>
/// When an owner-scoped vector search returns nothing, recall escalates: a widened probe over the
/// <b>global</b> index, then an owner-scoped scan. For an owner that holds no rows of that label both
/// are futile by construction — widening a global index cannot surface rows that do not exist — and
/// the widened probe is the expensive one, since it asks the index for up to <c>MaxTopK</c> candidates
/// across the entire corpus.
/// </para>
/// <para>
/// <b>The ladder still cannot simply be shortened, and that is what the measurement showed.</b> A
/// starved owner — plenty of rows, crowded out of the global top-K by noisier neighbours — <i>is</i>
/// recovered by the escalation, so removing a rung would lose real answers. The distinction the
/// optimisation rests on is not "did the search return nothing" but "does this owner have anything to
/// find", and only the second is answerable cheaply: it is bounded by one owner's data rather than by
/// the corpus.
/// </para>
/// <para>
/// <c>LIMIT 1</c> rather than a count: the question is existence, and counting an owner's whole
/// partition to learn it is non-zero would reintroduce the cost this avoids.
/// </para>
/// </remarks>
internal static class OwnerRowExistence
{
/// <summary>
/// Builds the existence probe for a label.
/// </summary>
/// <param name="label">Node label, e.g. <c>Fact</c>.</param>
/// <param name="includeShared">
/// Whether un-owned (shared) rows count. Must mirror the search's own scoping exactly — a probe
/// that judged the owner empty on stricter terms than the search would skip an escalation that
/// could have succeeded, which is a silent recall loss rather than a saving.
/// </param>
public static string Any(string label, bool includeShared)
{
var owner = includeShared
? "(n.owner_id = $ownerId OR n.owner_id IS NULL)"
: "n.owner_id = $ownerId";

// invalidated_at mirrors the vector searches, which all exclude soft-invalidated rows. An
// owner holding nothing but tombstones has nothing the escalation could return.
return $"""
MATCH (n:{label})
WHERE {owner} AND n.invalidated_at IS NULL
RETURN 1 AS present
LIMIT 1
""";
}
Comment on lines +39 to +53
}
39 changes: 38 additions & 1 deletion src/AgentMemory.Neo4j/Repositories/Neo4jEntityRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ internal sealed partial class Neo4jEntityRepository : IEntityRepository, IUpsert

private readonly INeo4jTransactionRunner _tx;
private readonly bool _rescueShortOwnerResults;
/// <summary>2.13: skip a futile widened probe + scan for an owner holding nothing.</summary>
private readonly bool _skipEscalationWhenOwnerHasNoRows;
/// <summary>Payload projection: drop the ~3 KB vector nothing on the recall path reads.</summary>
private readonly bool _omitEmbeddingsFromRecall;
private readonly ILogger<Neo4jEntityRepository> _logger;
Expand Down Expand Up @@ -68,6 +70,8 @@ public Neo4jEntityRepository(
IOptions<MemoryOptions>? memoryOptions = null)
{
_rescueShortOwnerResults = memoryOptions?.Value.RescueShortOwnerResults ?? false;
_skipEscalationWhenOwnerHasNoRows =
memoryOptions?.Value.SkipEscalationWhenOwnerHasNoRows ?? false;
_omitEmbeddingsFromRecall = memoryOptions?.Value.OmitEmbeddingsFromRecall ?? false;
_tx = tx;
_logger = logger;
Expand Down Expand Up @@ -256,7 +260,8 @@ public async Task<IReadOnlyList<Entity>> GetByNameAsync(
// this retry, so its absence here was a real exposure - an owner could receive NOTHING while
// its data sat in the graph.
int? escalatedTopK = null;
if (OwnerVectorOverFetch.ShouldEscalate(results.Count, hasOwner))
if (OwnerVectorOverFetch.ShouldEscalate(results.Count, hasOwner)
&& await ShouldClimbLadderAsync(scope, includeShared, cancellationToken).ConfigureAwait(false))
{
var widened = OwnerVectorOverFetch.EscalatedTopK(topK);
if (widened > topK)
Expand Down Expand Up @@ -1009,4 +1014,36 @@ internal static string SanitizeLabel(string label)
{
return new string(label.Where(c => char.IsLetterOrDigit(c) || c == '_').ToArray());
}

/// <summary>
/// Whether the escalation ladder can possibly help this owner (2.13).
/// </summary>
/// <remarks>
/// Returns <see langword="true"/> unless the option is on AND the owner provably holds no rows of
/// this label. Defaulting to "escalate" is the safe direction: a probe that wrongly reported empty
/// would skip a rescue that would have worked, and a silent recall loss costs far more than one
/// avoided query.
/// </remarks>
private async Task<bool> ShouldClimbLadderAsync(
MemoryScope? scope, bool includeShared, CancellationToken cancellationToken)
{
if (!_skipEscalationWhenOwnerHasNoRows || scope?.OwnerId is null) return true;

var present = await _tx.ReadAsync(async runner =>
{
var cursor = await runner.RunAsync(
OwnerRowExistence.Any("Entity", includeShared),
new { ownerId = scope.OwnerId }).ConfigureAwait(false);
return (await cursor.ToListAsync().ConfigureAwait(false)).Count > 0;
}, cancellationToken).ConfigureAwait(false);
Comment on lines +1032 to +1038

if (!present)
{
_logger.LogDebug(
"Owner {Owner} holds no Entity rows; skipping the escalation ladder (2.13).",
scope.OwnerId);
}

return present;
}
}
39 changes: 38 additions & 1 deletion src/AgentMemory.Neo4j/Repositories/Neo4jFactRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ internal sealed partial class Neo4jFactRepository : IFactRepository, IUpsertPers

private readonly INeo4jTransactionRunner _tx;
private readonly bool _rescueShortOwnerResults;
/// <summary>2.13: skip a futile widened probe + scan for an owner holding nothing.</summary>
private readonly bool _skipEscalationWhenOwnerHasNoRows;
/// <summary>Payload projection: drop the ~3 KB vector nothing on the recall path reads.</summary>
private readonly bool _omitEmbeddingsFromRecall;
private readonly double _reinforceAlpha;
Expand All @@ -41,6 +43,8 @@ public Neo4jFactRepository(
IOptions<MemoryOptions>? memoryOptions = null)
{
_rescueShortOwnerResults = memoryOptions?.Value.RescueShortOwnerResults ?? false;
_skipEscalationWhenOwnerHasNoRows =
memoryOptions?.Value.SkipEscalationWhenOwnerHasNoRows ?? false;
_omitEmbeddingsFromRecall = memoryOptions?.Value.OmitEmbeddingsFromRecall ?? false;
_reinforceAlpha = memoryOptions?.Value.ConfidenceReinforcementAlpha ?? 0.0;
_tx = tx;
Expand Down Expand Up @@ -417,7 +421,8 @@ await _tx.ReadAsync(async runner =>
// one wider query; anything non-empty is left alone, because escalating on "short" would tax
// every small tenant forever.
int? escalatedTopK = null;
if (OwnerVectorOverFetch.ShouldEscalate(results.Count, hasOwner))
if (OwnerVectorOverFetch.ShouldEscalate(results.Count, hasOwner)
&& await ShouldClimbLadderAsync(scope, includeShared, cancellationToken).ConfigureAwait(false))
{
var widened = OwnerVectorOverFetch.EscalatedTopK(topK);
if (widened > topK)
Expand Down Expand Up @@ -893,4 +898,36 @@ public async Task<IReadOnlyList<Fact>> SearchByCanonicalPredicatesAsync(
.ToList();
}, cancellationToken).ConfigureAwait(false);
}

/// <summary>
/// Whether the escalation ladder can possibly help this owner (2.13).
/// </summary>
/// <remarks>
/// Returns <see langword="true"/> unless the option is on AND the owner provably holds no rows of
/// this label. Defaulting to "escalate" is the safe direction: a probe that wrongly reported empty
/// would skip a rescue that would have worked, and a silent recall loss costs far more than one
/// avoided query.
/// </remarks>
private async Task<bool> ShouldClimbLadderAsync(
MemoryScope? scope, bool includeShared, CancellationToken cancellationToken)
{
if (!_skipEscalationWhenOwnerHasNoRows || scope?.OwnerId is null) return true;
Comment on lines +902 to +914

var present = await _tx.ReadAsync(async runner =>
{
var cursor = await runner.RunAsync(
OwnerRowExistence.Any("Fact", includeShared),
new { ownerId = scope.OwnerId }).ConfigureAwait(false);
return (await cursor.ToListAsync().ConfigureAwait(false)).Count > 0;
}, cancellationToken).ConfigureAwait(false);
Comment on lines +916 to +922

if (!present)
{
_logger.LogDebug(
"Owner {Owner} holds no Fact rows; skipping the escalation ladder (2.13).",
scope.OwnerId);
}

return present;
}
}
39 changes: 38 additions & 1 deletion src/AgentMemory.Neo4j/Repositories/Neo4jPreferenceRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ internal sealed partial class Neo4jPreferenceRepository : IPreferenceRepository,

private readonly INeo4jTransactionRunner _tx;
private readonly bool _rescueShortOwnerResults;
/// <summary>2.13: skip a futile widened probe + scan for an owner holding nothing.</summary>
private readonly bool _skipEscalationWhenOwnerHasNoRows;
/// <summary>Payload projection: drop the ~3 KB vector nothing on the recall path reads.</summary>
private readonly bool _omitEmbeddingsFromRecall;
private readonly ILogger<Neo4jPreferenceRepository> _logger;
Expand Down Expand Up @@ -68,6 +70,8 @@ public Neo4jPreferenceRepository(
IOptions<MemoryOptions>? memoryOptions = null)
{
_rescueShortOwnerResults = memoryOptions?.Value.RescueShortOwnerResults ?? false;
_skipEscalationWhenOwnerHasNoRows =
memoryOptions?.Value.SkipEscalationWhenOwnerHasNoRows ?? false;
_omitEmbeddingsFromRecall = memoryOptions?.Value.OmitEmbeddingsFromRecall ?? false;
_tx = tx;
_logger = logger;
Expand Down Expand Up @@ -268,7 +272,8 @@ public async Task<IReadOnlyList<Preference>> GetByCategoryAsync(
// 0 of the owner's 4 rows, and this retry restored all 4. Preferences run the identical
// global-index-then-post-filter shape, so the exposure was identical.
int? escalatedTopK = null;
if (OwnerVectorOverFetch.ShouldEscalate(results.Count, hasOwner))
if (OwnerVectorOverFetch.ShouldEscalate(results.Count, hasOwner)
&& await ShouldClimbLadderAsync(scope, includeShared, cancellationToken).ConfigureAwait(false))
{
var widened = OwnerVectorOverFetch.EscalatedTopK(topK);
if (widened > topK)
Expand Down Expand Up @@ -623,4 +628,36 @@ await runner.RunAsync(

return results;
}

/// <summary>
/// Whether the escalation ladder can possibly help this owner (2.13).
/// </summary>
/// <remarks>
/// Returns <see langword="true"/> unless the option is on AND the owner provably holds no rows of
/// this label. Defaulting to "escalate" is the safe direction: a probe that wrongly reported empty
/// would skip a rescue that would have worked, and a silent recall loss costs far more than one
/// avoided query.
/// </remarks>
private async Task<bool> ShouldClimbLadderAsync(
MemoryScope? scope, bool includeShared, CancellationToken cancellationToken)
{
if (!_skipEscalationWhenOwnerHasNoRows || scope?.OwnerId is null) return true;

var present = await _tx.ReadAsync(async runner =>
{
var cursor = await runner.RunAsync(
OwnerRowExistence.Any("Preference", includeShared),
new { ownerId = scope.OwnerId }).ConfigureAwait(false);
return (await cursor.ToListAsync().ConfigureAwait(false)).Count > 0;
}, cancellationToken).ConfigureAwait(false);
Comment on lines +646 to +652

if (!present)
{
_logger.LogDebug(
"Owner {Owner} holds no Preference rows; skipping the escalation ladder (2.13).",
scope.OwnerId);
}

return present;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ internal sealed class Neo4jReasoningTraceRepository : IReasoningTraceRepository
{
private readonly INeo4jTransactionRunner _tx;
private readonly bool _rescueShortOwnerResults;
/// <summary>2.13: skip a futile widened probe + scan for an owner holding nothing.</summary>
private readonly bool _skipEscalationWhenOwnerHasNoRows;
/// <summary>Scores this owner's OWN traces directly, bypassing the global vector index.</summary>
/// <remarks>
/// Extracted because two conditions reach it -- an empty scoped result, and (opt-in) a short one
Expand Down Expand Up @@ -60,6 +62,8 @@ public Neo4jReasoningTraceRepository(
IOptions<MemoryOptions>? memoryOptions = null)
{
_rescueShortOwnerResults = memoryOptions?.Value.RescueShortOwnerResults ?? false;
_skipEscalationWhenOwnerHasNoRows =
memoryOptions?.Value.SkipEscalationWhenOwnerHasNoRows ?? false;
_tx = tx;
_logger = logger;
}
Expand Down Expand Up @@ -251,7 +255,8 @@ public async Task<PagedResult<ReasoningTrace>> ListAllAsync(int limit = 50, int
// one wider query and cannot invent a match, so the retry is issued either way rather than
// guessing which cause applied.
int? escalatedTopK = null;
if (OwnerVectorOverFetch.ShouldEscalate(results.Count, hasOwner))
if (OwnerVectorOverFetch.ShouldEscalate(results.Count, hasOwner)
&& await ShouldClimbLadderAsync(scope, includeShared, cancellationToken).ConfigureAwait(false))
{
var widened = OwnerVectorOverFetch.EscalatedTopK(topK);
if (widened > topK)
Expand Down Expand Up @@ -545,4 +550,36 @@ private static ReasoningTrace MapToTrace(INode node, float[]? taskEmbedding) =>
// existing node at a different meaning.
["traceKind"] = trace.Kind == TraceKind.Procedure ? "procedure" : "episode"
};

/// <summary>
/// Whether the escalation ladder can possibly help this owner (2.13).
/// </summary>
/// <remarks>
/// Returns <see langword="true"/> unless the option is on AND the owner provably holds no rows of
/// this label. Defaulting to "escalate" is the safe direction: a probe that wrongly reported empty
/// would skip a rescue that would have worked, and a silent recall loss costs far more than one
/// avoided query.
/// </remarks>
private async Task<bool> ShouldClimbLadderAsync(
MemoryScope? scope, bool includeShared, CancellationToken cancellationToken)
{
if (!_skipEscalationWhenOwnerHasNoRows || scope?.OwnerId is null) return true;

var present = await _tx.ReadAsync(async runner =>
{
var cursor = await runner.RunAsync(
OwnerRowExistence.Any("ReasoningTrace", includeShared),
new { ownerId = scope.OwnerId }).ConfigureAwait(false);
return (await cursor.ToListAsync().ConfigureAwait(false)).Count > 0;
}, cancellationToken).ConfigureAwait(false);
Comment on lines +568 to +574

if (!present)
{
_logger.LogDebug(
"Owner {Owner} holds no ReasoningTrace rows; skipping the escalation ladder (2.13).",
scope.OwnerId);
}

return present;
}
}
Loading