diff --git a/src/Netclaw.Actors.Tests/Memory/MemoryCurationEvaluatorParityTests.cs b/src/Netclaw.Actors.Tests/Memory/MemoryCurationEvaluatorParityTests.cs index b0f00fcb1..6510f3867 100644 --- a/src/Netclaw.Actors.Tests/Memory/MemoryCurationEvaluatorParityTests.cs +++ b/src/Netclaw.Actors.Tests/Memory/MemoryCurationEvaluatorParityTests.cs @@ -175,10 +175,19 @@ await SeedDocumentAsync( Assert.Equal(CurationDecisionKind.Create, fromActor.Kind); } - // ── guard downgrade: Update whose proposal drops existing body -> Skip ── + // ── guard downgrade: Update whose proposal drops existing body -> falls through ── + /// + /// Guard-fallthrough fix (July 2026 audit, eval run ad9a2312): before this fix, a + /// guard-rejected anchor-matched Update terminated as Skip — an explicit proposal silently + /// becoming a no-op with zero writes. No other candidate exists in this store for the + /// fallen-through content search or embedding nominator (both unavailable/empty here) to + /// find, so the terminal decision is the Create default the flow's remarks document — NOT + /// the old Skip. This is the same fixture GuardDowngrade_narrowerProposal_downgradesUpdateToSkip_identically + /// used pre-fix (renamed here since Skip was exactly the bug). + /// [Fact] - public async Task GuardDowngrade_narrowerProposal_downgradesUpdateToSkip_identically() + public async Task GuardDowngrade_narrowerProposal_noOtherCandidates_fallsThroughToCreate_identically() { var ct = TestContext.Current.CancellationToken; await _store.InitializeAsync(ct); @@ -189,19 +198,169 @@ await SeedDocumentAsync( freshnessAtMs: 1000, ct); - // Newer, but narrower — the rules tier would pick Update (exact anchor, low - // overlap, fresher), and GuardDestructiveUpdate must downgrade it on BOTH paths - // now (audit finding D14: this guard used to run only on the inline actor path). + // Newer, but narrower — the rules tier would pick Update (exact anchor, low overlap, + // fresher), and GuardDestructiveUpdate downgrades it on BOTH paths (audit finding D14: + // this guard used to run only on the inline actor path). Pre-fix, that downgrade was + // returned as the terminal Skip decision — the silent-fallback bug. Post-fix, the guard + // rejection triggers a fall-through re-evaluation (no exact anchor match, nomination/ + // content search run for the first time, rules tier re-runs as pure fuzzy) which here + // finds nothing else to match against and lands on Create. var operation = MakeOperation("widget-specs", "Widget pricing is TBD as of Q2.", freshnessAtMs: 2000); var (fromActor, fromEngine) = await EvaluateOnBothAsync(operation, ct); + AssertSameDecision(fromActor, fromEngine); + Assert.Equal(CurationDecisionKind.Create, fromActor.Kind); + Assert.Contains("fuzzy anchor match but low content overlap", fromActor.Reason); + } + + /// + /// Regression companion to the Create case above: when the guard-rejected proposal IS close + /// enough in content to the anchor target to clear the deterministic auto-resolve thresholds + /// ('s 60% content-overlap / 50% + /// anchor-Jaccard bars) once re-evaluated as an ordinary fuzzy candidate, the fall-through + /// must NOT force Create — it must let the normal ambiguous/auto-resolve machinery decide, + /// which correctly lands on Skip here. This proves the fix doesn't trade "always drops the + /// fact" for "never skips a real duplicate" — the fall-through defers to whatever the rest of + /// the flow genuinely produces. + /// + [Fact] + public async Task GuardDowngrade_contentCloseEnoughToAutoResolve_fallsThroughToLegitimateSkip_identically() + { + var ct = TestContext.Current.CancellationToken; + await _store.InitializeAsync(ct); + await SeedDocumentAsync( + "widget-specs", + "doc-widget", + "Widget specs: 16 cores, 64GB RAM, 2 NICs. Warranty is 3 years from Acme Corp in Denver.", + freshnessAtMs: 1000, + ct); + + // Reworded/reordered restatement of the SAME facts: word-level overlap is ~62% (inside + // the exact-match tier's Update band, ≤80%) but GuardDestructiveUpdate's stricter + // substring-containment check fails (the words are reordered, not a literal superset), + // so the guard still rejects. Re-evaluated as a fuzzy candidate after fall-through, that + // same ~62% overlap clears TryAutoResolveAmbiguous's 60% content / 100% anchor-Jaccard + // (identical anchor name) thresholds, so this is genuine skip territory rather than the + // guard's blunt termination. + var operation = MakeOperation( + "widget-specs", + "Acme Corp widget in Denver: 2 NICs, 64GB RAM, 16 cores, and a 3 year warranty included.", + freshnessAtMs: 2000); + + var (fromActor, fromEngine) = await EvaluateOnBothAsync(operation, ct); + AssertSameDecision(fromActor, fromEngine); Assert.Equal(CurationDecisionKind.Skip, fromActor.Kind); - Assert.Contains("update guarded", fromActor.Reason); + Assert.Contains("auto-resolved", fromActor.Reason); Assert.Equal("doc-widget", fromActor.TargetDocumentId); } + /// + /// Asserts the structured curation_guard_fallthrough marker fires with the rejected + /// anchor and target — the July 2026 audit tooling greps daemon logs for this exact string + /// (per this class's remarks), so the marker itself is a load-bearing observability + /// contract, not incidental. + /// + [Fact] + public async Task GuardDowngrade_logsStructuredFallthroughMarker() + { + var ct = TestContext.Current.CancellationToken; + await _store.InitializeAsync(ct); + await SeedDocumentAsync( + "widget-specs", + "doc-widget", + "Widget specs: 16 cores, 64GB RAM, 2 NICs. Pricing on file. Vendor contacts listed.", + freshnessAtMs: 1000, + ct); + + var operation = MakeOperation("widget-specs", "Widget pricing is TBD as of Q2.", freshnessAtMs: 2000); + + var recordingLogger = new RecordingLogger(); + var evaluator = new MemoryCurationEvaluator(_store, (ILogger)recordingLogger, new MemoryCurationConfig()); + + var evaluation = await evaluator.EvaluateAsync(operation, TestSessionId, ct); + + Assert.Equal(CurationDecisionKind.Create, evaluation.Decision.Kind); + Assert.Contains( + recordingLogger.Entries, + e => e.Contains("curation_guard_fallthrough", StringComparison.Ordinal) + && e.Contains("widget-specs", StringComparison.Ordinal) + && e.Contains("doc-widget", StringComparison.Ordinal)); + } + + // ── guard downgrade + nominator: near-dupe elsewhere still forces the LLM tier ── + + /// + /// A guard-rejected anchor Update must not merely fall through to Create/auto-resolve when a + /// real embedding nominee is available — the fall-through re-runs nomination (this proposal's + /// exact anchor match previously short-circuited it entirely, so it had never run at all), and + /// a nominee at or above must + /// still force the LLM tier exactly as it would for a proposal with no anchor match in the + /// first place (design D4: cosine nominates, it never auto-decides). + /// + [Fact] + public async Task GuardDowngrade_withNominatorNearDupe_forcesLlmTier_identically() + { + var ct = TestContext.Current.CancellationToken; + await _store.InitializeAsync(ct); + + // Same anchor-collision shape as the eval-run repro: an exact anchor match whose content + // is unrelated to the proposal (guard will reject the Update), PLUS a real near-duplicate + // elsewhere in the store that only the embedding nominator — never run on the first pass + // because the exact anchor match short-circuited it — can find. + await SeedDocumentAsync( + "the", + "doc-unrelated-junk-anchor", + "Unrelated content that happens to share the same junk anchor name.", + freshnessAtMs: 1000, + ct); + + const string nearDupeBody = "The build pipeline stores intermediate render artifacts in a graphite-backed cache layer."; + var nearDupeAnchor = _store.CreateDefaultAnchor("graphite-render-cache"); + await _store.UpsertDocumentAsync(new SQLiteMemoryDocument( + DocumentId: "doc-near-dupe", + Anchor: nearDupeAnchor, + MemoryClass: "durable_fact", + Title: "Existing near-dupe", + MarkdownBody: nearDupeBody, + AliasesJson: null, + FacetsJson: null, + SlotsJson: null, + UpdateSemantics: "merge-document", + Sensitivity: "normal", + RecallMode: "auto", + Confidence: 0.9, + FreshnessAtMs: 1000, + ExpiresAtMs: null, + CreatedAtMs: 1000, + UpdatedAtMs: 1000), ct); + await _store.UpsertEmbeddingAsync( + "doc-near-dupe", MemoryEmbedOnWriteCoordinator.DocumentItemKind, "test-nominator-model", "hash-near-dupe", + new float[] { 1f, 0f }, ct); + + var operation = MakeOperation( + "the", "Deployment jobs wait in a queue before promotion to production.", freshnessAtMs: 2000); + + var embedderHolder = new MemoryEmbedderHolder( + new ScriptedEmbedder("test-nominator-model", dimensions: 2, [0.93f, 0.367623f])); + var vectorIndexHolder = new MemoryVectorIndexHolder(_store); + + var actorLike = new MemoryCurationEvaluator( + _store, (ILoggingAdapter)NoLogger.Instance, new MemoryCurationConfig(), + new ScriptedCurationChatClient("SKIP"), embedderHolder, vectorIndexHolder); + var engineLike = new MemoryCurationEvaluator( + _store, (ILogger)NullLogger.Instance, new MemoryCurationConfig(), + new ScriptedCurationChatClient("SKIP"), embedderHolder, vectorIndexHolder); + + var fromActor = (await actorLike.EvaluateAsync(operation, TestSessionId, ct)).Decision; + var fromEngine = (await engineLike.EvaluateAsync(operation, TestSessionId, ct)).Decision; + + AssertSameDecision(fromActor, fromEngine); + Assert.True(fromActor.FromLlmTier); + Assert.Equal(CurationDecisionKind.Skip, fromActor.Kind); + } + // ── LLM tier: parseable decision ───────────────────────────────── [Fact] @@ -450,4 +609,25 @@ public ValueTask>> EmbedBatchAsync(IReadOnly => ValueTask.FromResult>>( texts.Select(_ => (ReadOnlyMemory)queryVector).ToList()); } + + /// + /// Records every log line emitted through the Microsoft.Extensions.Logging ctor path, so the + /// curation_guard_fallthrough marker can be asserted directly rather than only + /// inferred from the resulting decision shape. Mirrors + /// MemoryCurationNominatorTests.RecordingLogger (kept as a separate private copy per + /// that file's own convention for test-only doubles). + /// + private sealed class RecordingLogger : ILogger + { + public List Entries { get; } = []; + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(Microsoft.Extensions.Logging.LogLevel logLevel) => true; + + public void Log( + Microsoft.Extensions.Logging.LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + => Entries.Add(formatter(state, exception)); + } } diff --git a/src/Netclaw.Actors/Memory/MemoryCurationEvaluator.cs b/src/Netclaw.Actors/Memory/MemoryCurationEvaluator.cs index cf3e393a8..ee31e2402 100644 --- a/src/Netclaw.Actors/Memory/MemoryCurationEvaluator.cs +++ b/src/Netclaw.Actors/Memory/MemoryCurationEvaluator.cs @@ -21,7 +21,7 @@ namespace Netclaw.Actors.Memory; /// curation_nominator_degraded, curation_nominee_no_llm_decision, /// curation_llm_decision, curation_llm_no_decision, curation_llm_timeout, /// curation_llm_error, curation_ambiguous_auto_resolved, -/// curation_ambiguous_create_fallback, +/// curation_ambiguous_create_fallback, curation_guard_fallthrough, /// curation_skip/_update/_consolidate/_create, /// curation_reanchor, curation_tombstone_anchor) regardless of which of the /// two callers is driving the evaluator: the inline per-session actor @@ -79,24 +79,48 @@ internal sealed class MicrosoftCurationLog(ILogger log) : ICurationLog /// Decision flow (): immutable records bypass evaluation; fuzzy /// anchor candidates are queried; on an exact anchor match, the deterministic fast path /// ('s EvaluateExactMatch) decides Skip/Update -/// with no further evidence gathering. Otherwise, the embedding kNN nominator (memory-core- -/// redesign Slice 3 Stage B, task 3.1, design D4) queries -/// when the embedder is available: any nominee forces the decision to the LLM tier, -/// regardless of what the lexical rules tier would have decided — the May 2026 measurement -/// (docs/research/memory-recall-findings-2026-05.md; corroborated at corpus scale in -/// docs/research/memory-audit-2026-07.md §5) found no cosine threshold that separates true -/// duplicates from merely-related siblings, so cosine similarity is nomination evidence ONLY — -/// it never auto-merges and never auto-skips. When no nominee fires (or the embedder is -/// unavailable, in which case the pre-Slice-3 lexical content-term search runs instead as an -/// explicitly-logged degraded path), runs the -/// deterministic tier as before; an Ambiguous result escalates to the LLM tier (when available) -/// followed by , or else falls back to -/// and finally a Create default. -/// maps the resulting decision to the operation that should -/// be written (or nothing, for Skip), executing Consolidate's re-anchor/tombstone side +/// with no further evidence gathering — UNLESS that Update is downgraded by +/// (see "Guard fall-through" below), +/// in which case evidence gathering resumes rather than terminating. Otherwise, the embedding +/// kNN nominator (memory-core-redesign Slice 3 Stage B, task 3.1, design D4) queries +/// when the embedder is available: any nominee forces the +/// decision to the LLM tier, regardless of what the lexical rules tier would have decided — +/// the May 2026 measurement (docs/research/memory-recall-findings-2026-05.md; corroborated +/// at corpus scale in docs/research/memory-audit-2026-07.md §5) found no cosine threshold +/// that separates true duplicates from merely-related siblings, so cosine similarity is +/// nomination evidence ONLY — it never auto-merges and never auto-skips. When no nominee fires +/// (or the embedder is unavailable, in which case the pre-Slice-3 lexical content-term search +/// runs instead as an explicitly-logged degraded path), +/// runs the deterministic tier as before; an Ambiguous result escalates to the LLM tier (when +/// available) followed by , or else +/// falls back to and finally a Create +/// default. maps the resulting decision to the operation that +/// should be written (or nothing, for Skip), executing Consolidate's re-anchor/tombstone side /// effects — this mapping is unified too, since a second, hand-copied switch statement per /// caller is exactly the kind of divergence this slice removes. /// +/// +/// Guard fall-through (memory-core-redesign, July 2026 audit finding, eval run +/// ad9a2312): when the exact-anchor deterministic path picks Update and +/// downgrades it to Skip because the +/// proposal would not preserve the target's content, that Skip must NOT be returned as the final +/// decision — an explicit store_memory proposal silently ending as a no-op (no write, no +/// further evidence gathering) is a silent-fallback violation, observed when an LLM-emitted junk +/// anchor (e.g. the bare stopword the) collided with an unrelated existing document sharing +/// the same junk anchor text. The guard's protective effect is correct and must be kept — the +/// mismatched target must never be overwritten — but the correct response to "this anchor match +/// doesn't apply" is to re-run evaluation exactly as if there had been no exact anchor match at +/// all: every candidate that carried IsExactAnchorMatch is demoted to an ordinary fuzzy +/// candidate (so the rules tier cannot re-derive the same Update/guard-reject pair and loop), then +/// the embedding nominator / lexical content-term search that the exact-match fast path had +/// short-circuited runs for the first time, followed by the usual rules-tier/LLM-tier/auto-resolve +/// chain with a Create default. This is deliberately NOT a fix to anchor-name hygiene — junk +/// anchors like the should arguably never be stored or fuzzy-matched at all, but filtering +/// them is a broader behavior change to anchor matching left as a follow-up; this fall-through +/// fixes the narrower "explicit store becomes a silent no-op" failure regardless of why the guard +/// rejected the match. +/// +/// /// Guard-validated write routing (memory-core-redesign Slice 3, design D5): an LLM-tier /// UPDATE/CONSOLIDATE decision () never overwrites /// a target's raw body. When it carries a synthesized , @@ -221,12 +245,38 @@ public async Task EvaluateAsync( // Build a mutable candidate list — content search may add more candidates below. var candidates = new List(anchorCandidates); + return await EvaluateCandidatesAsync( + operation, sessionId, candidates, anchorCandidates.Any(c => c.IsExactAnchorMatch), ct); + } + + /// + /// The evaluation body proper, factored out of so the guard + /// fall-through case (see this class's remarks) can re-run the same evidence-gathering and + /// decision chain a second time with forced false — + /// exactly as if the anchor query at the top of had found no + /// exact match — without re-querying the store for anchors a second time. + /// + private async Task EvaluateCandidatesAsync( + SQLiteMemoryCurationOperation operation, + SessionId sessionId, + List candidates, + bool hasExactAnchorMatch, + CancellationToken ct) + { // Embedding kNN nomination (memory-core-redesign Slice 3 Stage B, task 3.1, design D4) // vs. the pre-Slice-3 lexical content-term search: these are alternatives, not additive. // An exact anchor match already resolves deterministically below with no further // evidence gathering (the "existing exact-anchor deterministic fast path" design D4 - // calls out as unchanged), so neither runs in that case. - var hasExactAnchorMatch = anchorCandidates.Any(c => c.IsExactAnchorMatch); + // calls out as unchanged), so neither runs in that case — unless the guard fall-through + // below re-invokes this method with hasExactAnchorMatch forced false, in which case this + // runs for the first time for this proposal. + // + // Captured before any mutation below: on the first (normal) call this is exactly the + // anchor-name-fuzzy-match hit count `curation_dual_search` below reports; on a guard + // fall-through re-entry it is that same anchor-hit count with the rejected match(es) + // demoted rather than removed, which is the correct "anchor_hits" figure for this pass + // either way (no nomination/content-search candidates have been merged in yet). + var anchorHitCount = candidates.Count; if (!hasExactAnchorMatch) { var embedder = _embedderHolder?.Current; @@ -308,7 +358,7 @@ public async Task EvaluateAsync( _log.Debug( "curation_dual_search anchor={0} anchor_hits={1} content_hits={2} merged={3}", operation.AnchorCanonicalName, - anchorCandidates.Count, + anchorHitCount, contentCandidates.Count, candidates.Count); } @@ -409,9 +459,38 @@ public async Task EvaluateAsync( candidates); } - return new CurationEvaluation( - CurationRulesEvaluator.GuardDestructiveUpdate(rulesDecision, operation, candidates), - candidates); + var guardedDecision = CurationRulesEvaluator.GuardDestructiveUpdate(rulesDecision, operation, candidates); + + // Guard fall-through (see this class's remarks): the ONLY decision shape + // GuardDestructiveUpdate ever changes is Update -> Skip, and Update can only be + // produced by the exact-anchor deterministic fast path (CurationRulesEvaluator's fuzzy + // tier never returns Update) — so this downgrade is only reachable when + // hasExactAnchorMatch was true for this call. Terminating on that Skip would silently + // drop an explicit proposal with no further evidence gathering (the July 2026 audit + // bug); instead, demote every exact-anchor-matched candidate to an ordinary fuzzy + // candidate and re-run the full evaluation chain as if there had been no exact anchor + // match at all. The demotion is what keeps this from looping: with no candidate left + // claiming IsExactAnchorMatch, CurationRulesEvaluator.Evaluate cannot re-derive the same + // Update decision on the re-run, so this branch cannot fire twice for one proposal. + if (hasExactAnchorMatch + && rulesDecision.Kind == CurationDecisionKind.Update + && guardedDecision.Kind == CurationDecisionKind.Skip) + { + _log.Warning( + "curation_guard_fallthrough anchor={0} rejectedTarget={1}", + operation.AnchorCanonicalName, + guardedDecision.TargetDocumentId ?? "(unknown)"); + + for (var i = 0; i < candidates.Count; i++) + { + if (candidates[i].IsExactAnchorMatch) + candidates[i] = candidates[i] with { IsExactAnchorMatch = false }; + } + + return await EvaluateCandidatesAsync(operation, sessionId, candidates, hasExactAnchorMatch: false, ct); + } + + return new CurationEvaluation(guardedDecision, candidates); } ///