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
160 changes: 160 additions & 0 deletions tests/Tests.FSharp/Operators/RecursiveSemiNaive.Boundary.Tests.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
module Zeta.Tests.Operators.RecursiveSemiNaiveBoundaryTests

open FsUnit.Xunit
open global.Xunit
open Zeta.Core


// ═══════════════════════════════════════════════════════════════════
// Boundary tests for `RecursiveSemiNaive`.
//
// Encodes the two scenarios from `openspec/specs/retraction-safe-
// recursion/spec.md` § "Requirement: semi-naïve recursion is
// monotone-only":
//
// Scenario 1: monotone inputs yield the same result as the
// retraction-safe combinator.
// Scenario 2: retraction leaks stale facts (the documented
// boundary; fed retracting input, semi-naïve's
// integrated output retains rows derived from the
// retracted fact).
//
// Motivation: Amara's 2026-04-23 ZSet-semantics courier report
// (absorbed as `docs/aurora/2026-04-23-amara-zset-semantics-
// operator-algebra.md` via PR #211) called out the
// `RecursiveSemiNaive` monotone-only boundary as a "must remain
// explicitly labeled" gap. Code and spec already document it; this
Comment on lines +22 to +26
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: This file’s header comment includes direct contributor name attribution (“Amara's …”). Repo policy forbids contributor names in code/docs/skills outside memory/persona/** (see docs/AGENT-BEST-PRACTICES.md operational rules). Please rewrite this motivation text using a role reference (e.g., “courier report” / “external semantics report”) without naming the contributor.

Copilot uses AI. Check for mistakes.
// file adds the *executable* documentation — the tests assert the
// monotone-equivalent behavior AND the retraction-leak behavior as
// the current documented boundary. If a future change fixes the
// leak without also fixing the algorithm (see the gap-monotone
// signed-delta research plan in `docs/research/retraction-safe-
// semi-naive.md`), the second test will fail and prompt an
// investigation.
//
// Reading: *the leak test is not a bug being asserted; it is a
// boundary being recorded.* Callers that want retraction safety
// already have `Recursive` available. This suite protects
// `RecursiveSemiNaive`'s documented semantics from silent drift.
// ═══════════════════════════════════════════════════════════════════


/// Transitive-closure one-step body over `(u,v)` edges: given the
/// current `reach` set, produce additional `(a,z)` reach via a
/// key-join between `reach.2` and `edges.1`. The body is Z-linear.
let private oneStepClosure (c: Circuit) (edges: Stream<ZSet<struct (int * int)>>)
(reach: Stream<ZSet<struct (int * int)>>) : Stream<ZSet<struct (int * int)>> =
c.Join(
reach,
edges,
System.Func<_, _>(fun (struct (_, m)) -> m),
System.Func<_, _>(fun (struct (u, _)) -> u),
System.Func<_, _, _>(fun (struct (a, _)) (struct (_, z)) ->
struct (a, z)))


// ─── Scenario 1: monotone inputs match the retraction-safe combinator ───

[<Fact>]
let ``RecursiveSemiNaive matches Recursive on monotone inputs (acyclic DAG)`` () =
// A --> B --> C
// Monotone: only inserts, no retractions.
// Closure: {(1,2), (2,3), (1,3)} once fully expanded.
let edges = [ struct (1, 2); struct (2, 3) ]

// ── Retraction-safe combinator reference ──
let cRef = Circuit()
let edgesRef = cRef.ZSetInput<struct (int * int)>()
let closureRef =
cRef.Recursive(
edgesRef.Stream,
System.Func<_, _>(fun s -> oneStepClosure cRef edgesRef.Stream s))
let outRef = OutputHandle closureRef.Op
cRef.Build()
edgesRef.Send (ZSet.ofKeys edges)
let struct (_, _) = cRef.IterateToFixedPointWithConvergence(closureRef, 20)
let refResult = outRef.Current
Comment on lines +74 to +76
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: IterateToFixedPointWithConvergence returns a converged flag but it’s ignored here. Other tests in this repo assert convergence; please capture the boolean and assert it’s True so the test can’t silently pass with a non-converged intermediate state.

Copilot uses AI. Check for mistakes.

// ── Semi-naïve under test ──
let cSN = Circuit()
let edgesSN = cSN.ZSetInput<struct (int * int)>()
let closureSN =
cSN.RecursiveSemiNaive(
edgesSN.Stream,
System.Func<_, _>(fun s -> oneStepClosure cSN edgesSN.Stream s))
let outSN = OutputHandle closureSN.Op
cSN.Build()
edgesSN.Send (ZSet.ofKeys edges)
let struct (_, _) = cSN.IterateToFixedPointWithConvergence(closureSN, 20)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Assert convergence before reading fixed-point outputs

IterateToFixedPointWithConvergence returns whether the iteration cap was hit before stabilization, but this result is discarded. Because both scenarios are defined in terms of behavior after the operators have stabilized, the test can silently validate transient state if a future change causes non-convergence within 20 steps, reducing its ability to catch real regressions.

Useful? React with 👍 / 👎.

let snResult = outSN.Current
Comment on lines +87 to +89
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: IterateToFixedPointWithConvergence’s convergence result is discarded here as well. Please assert converged = true (or raise the iteration cap with an explanation) to avoid comparing outputs from a non-fixed-point state.

Copilot uses AI. Check for mistakes.

// Both combinators MUST agree on the integrated closure under
// monotone input. This is spec scenario #1.
//
// We check by positive-weight support equality: both should
// contain exactly {(1,2), (2,3), (1,3)} with positive weight.
// (Weight magnitudes may differ — `Recursive` uses `Distinct`
// internally so all weights clamp to 1; `RecursiveSemiNaive`
// may carry higher counts.)
Comment on lines +96 to +98
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: The comment claims RecursiveSemiNaive “may carry higher counts” than Recursive, but RecursiveSemiNaive uses Distinct inside the loop (same as Recursive), so output weights are clamped to {0,1}. Please fix or remove this misleading explanation.

Suggested change
// (Weight magnitudes may differ — `Recursive` uses `Distinct`
// internally so all weights clamp to 1; `RecursiveSemiNaive`
// may carry higher counts.)
// In this case the comparison is about the reachable support,
// not differing multiplicities: both combinators clamp output
// weights to positive support here.

Copilot uses AI. Check for mistakes.
let supportOf (z: ZSet<struct (int * int)>) =
z
|> Seq.filter (fun (e: ZEntry<struct (int * int)>) -> e.Weight > 0L)
|> Seq.map (fun e -> e.Key)
|> Set.ofSeq

supportOf snResult |> should equal (supportOf refResult)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Enforce weight equality in monotone equivalence test

This scenario is meant to encode the spec requirement that RecursiveSemiNaive and Recursive “agree” after stabilization, but the assertion only compares positive-key support and ignores Z-weights. That means a regression where semi-naïve returns incorrect weights (for example, if distinct-clamping is accidentally removed) would still pass as long as the same keys are present, so the test would not actually guard the stated contract.

Useful? React with 👍 / 👎.



Comment on lines +105 to +107
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: This assertion only checks that Recursive and RecursiveSemiNaive agree with each other; it would still pass if both were wrong in the same way (e.g., both empty). Please also assert the expected closure support explicitly ((1,2), (2,3), (1,3)) and then check both results against that oracle.

Suggested change
supportOf snResult |> should equal (supportOf refResult)
let expectedSupport =
set [
struct (1, 2)
struct (2, 3)
struct (1, 3)
]
let refSupport = supportOf refResult
let snSupport = supportOf snResult
refSupport |> should equal expectedSupport
snSupport |> should equal expectedSupport
snSupport |> should equal refSupport

Copilot uses AI. Check for mistakes.
// ─── Scenario 2: retraction leaks stale facts (documented boundary) ─────

[<Fact>]
let ``RecursiveSemiNaive leaks stale facts after retraction (documented boundary)`` () =
// tick 0: insert edge (1,2) then (2,3) → closure {(1,2), (2,3), (1,3)}
// tick 1: retract edge (2,3) → closure should become {(1,2)} if retraction-safe.
//
// Under `Recursive` (retraction-safe) the closure row (1,3)
// drops out. Under `RecursiveSemiNaive` (monotone-only) the
// row (1,3) LEAKS — it was written to the internal `total`
// feedback cell and the algorithm has no path to reverse it.
//
// This test ASSERTS the leak. It is the documented boundary,
// not a bug being fixed. See `openspec/specs/retraction-safe-
// recursion/spec.md` § "Scenario: retraction leaks stale facts"
// for the spec-level SHALL-NOT equivalent.
//
// If this test ever *fails* because (1,3) drops out correctly
// after retraction, the fix is either:
// (a) replace the combinator with the gap-monotone signed-
// delta variant (ongoing research;
// `docs/research/retraction-safe-semi-naive.md`), and
// remove this test's leak-assertion; OR
// (b) investigate whether the test harness now masks the
// leak via some other operator's behavior, and adjust
// the scenario to surface it directly.

// ── Semi-naïve under test ──
let cSN = Circuit()
let edgesSN = cSN.ZSetInput<struct (int * int)>()
let closureSN =
cSN.RecursiveSemiNaive(
edgesSN.Stream,
System.Func<_, _>(fun s -> oneStepClosure cSN edgesSN.Stream s))
let outSN = OutputHandle closureSN.Op
cSN.Build()

// tick 0: inserts (1,2), (2,3) → closure grows to include (1,3).
edgesSN.Send (ZSet.ofKeys [ struct (1, 2); struct (2, 3) ])
let struct (_, _) = cSN.IterateToFixedPointWithConvergence(closureSN, 20)
let after0 = outSN.Current
after0.[struct (1, 3)] |> should be (greaterThan 0L)

// tick 1: retract edge (2,3). Under semi-naïve, the positive-
// integrated `total` cannot be reversed — (1,3) leaks.
edgesSN.Send (ZSet.ofPairs [ struct (struct (2, 3), -1L) ])
let struct (_, _) = cSN.IterateToFixedPointWithConvergence(closureSN, 20)
Comment on lines +147 to +154
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Convergence is ignored here too (IterateToFixedPointWithConvergence). Please assert the returned converged flag so the leak assertion is evaluated on the stabilized output (and so failures diagnose cap/termination issues clearly).

Suggested change
let struct (_, _) = cSN.IterateToFixedPointWithConvergence(closureSN, 20)
let after0 = outSN.Current
after0.[struct (1, 3)] |> should be (greaterThan 0L)
// tick 1: retract edge (2,3). Under semi-naïve, the positive-
// integrated `total` cannot be reversed — (1,3) leaks.
edgesSN.Send (ZSet.ofPairs [ struct (struct (2, 3), -1L) ])
let struct (_, _) = cSN.IterateToFixedPointWithConvergence(closureSN, 20)
let struct (_, converged0) = cSN.IterateToFixedPointWithConvergence(closureSN, 20)
converged0 |> should equal true
let after0 = outSN.Current
after0.[struct (1, 3)] |> should be (greaterThan 0L)
// tick 1: retract edge (2,3). Under semi-naïve, the positive-
// integrated `total` cannot be reversed — (1,3) leaks.
edgesSN.Send (ZSet.ofPairs [ struct (struct (2, 3), -1L) ])
let struct (_, converged1) = cSN.IterateToFixedPointWithConvergence(closureSN, 20)
converged1 |> should equal true

Copilot uses AI. Check for mistakes.
Comment on lines +147 to +154
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Same issue on the retraction tick: the converged flag is discarded. Please assert convergence (or document/raise the cap) before reading outSN.Current so the test can’t accidentally validate an intermediate state.

Suggested change
let struct (_, _) = cSN.IterateToFixedPointWithConvergence(closureSN, 20)
let after0 = outSN.Current
after0.[struct (1, 3)] |> should be (greaterThan 0L)
// tick 1: retract edge (2,3). Under semi-naïve, the positive-
// integrated `total` cannot be reversed — (1,3) leaks.
edgesSN.Send (ZSet.ofPairs [ struct (struct (2, 3), -1L) ])
let struct (_, _) = cSN.IterateToFixedPointWithConvergence(closureSN, 20)
let struct (_, converged0) = cSN.IterateToFixedPointWithConvergence(closureSN, 20)
Assert.True(converged0, "Semi-naïve closure did not converge within 20 iterations on tick 0.")
let after0 = outSN.Current
after0.[struct (1, 3)] |> should be (greaterThan 0L)
// tick 1: retract edge (2,3). Under semi-naïve, the positive-
// integrated `total` cannot be reversed — (1,3) leaks.
edgesSN.Send (ZSet.ofPairs [ struct (struct (2, 3), -1L) ])
let struct (_, converged1) = cSN.IterateToFixedPointWithConvergence(closureSN, 20)
Assert.True(converged1, "Semi-naïve closure did not converge within 20 iterations on the retraction tick.")

Copilot uses AI. Check for mistakes.
let after1 = outSN.Current

// Leaked row (1,3) MUST still carry positive weight — this
// is the documented boundary. If it drops to 0, the test
// fails and prompts the investigation above.
after1.[struct (1, 3)] |> should be (greaterThan 0L)
1 change: 1 addition & 0 deletions tests/Tests.FSharp/Tests.FSharp.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
<Compile Include="Operators/ResidualMax.Tests.fs" />
<Compile Include="Operators/Differentiate.Tests.fs" />
<Compile Include="Operators/RecursiveCounting.MultiSeed.Tests.fs" />
<Compile Include="Operators/RecursiveSemiNaive.Boundary.Tests.fs" />

<!-- Storage/ -->
<Compile Include="Storage/Spine.Tests.fs" />
Expand Down
Loading