diff --git a/Directory.Build.props b/Directory.Build.props index be6b2ec4f..3a3f1e3e1 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -9,7 +9,7 @@ enable true 0.25.0 - beta.1 + beta.2 Netclaw v0.25.0-beta.1 — SkillServer native sub-agent sync, memory curation unification, systemd PATH fix **Features** diff --git a/Directory.Packages.props b/Directory.Packages.props index bd52f917b..88d7af217 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -67,14 +67,14 @@ - + - + diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 2e5e293d9..0f4a5cffb 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,5 +1,14 @@ # NetClaw Release Notes +## 0.25.0-beta.2 (2026-07-07) + +### Bug Fixes +- **UTF-8 BOM in skill frontmatter** — Fixed: skill scanner now strips UTF-8 BOM (`\uFEFF`) before parsing YAML frontmatter, and populates `SkillName` on all `SkillScanIssue` records so degenerate frontmatter no longer crashes the scan ([#1583](https://github.com/netclaw-dev/netclaw/pull/1583)) +- **Model capability provenance logging** — Fixed: daemon now logs effective model capabilities with their provenance source, improving diagnostic visibility for model configuration issues ([#1584](https://github.com/netclaw-dev/netclaw/pull/1584)) + +### Dependency Updates +- **Bump SkillServer** — `Netclaw.SkillClient` 0.4.0-beta.1 → 0.4.0-beta.3 and adapt to API changes ([#1593](https://github.com/netclaw-dev/netclaw/pull/1593)) + ## 0.25.0-beta.1 (2026-07-05) ### Features diff --git a/openspec/changes/memory-relevance-gate/.openspec.yaml b/openspec/changes/memory-relevance-gate/.openspec.yaml new file mode 100644 index 000000000..dd9a1d92e --- /dev/null +++ b/openspec/changes/memory-relevance-gate/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-06 diff --git a/openspec/changes/memory-relevance-gate/design.md b/openspec/changes/memory-relevance-gate/design.md new file mode 100644 index 000000000..5ae6e76ea --- /dev/null +++ b/openspec/changes/memory-relevance-gate/design.md @@ -0,0 +1,387 @@ +# Design: memory-relevance-gate + +## Context + +memory-core-redesign Slice 4 (`openspec/changes/memory-core-redesign/`, design +D6) shipped hybrid recall with an absolute cosine floor +(`Memory.Recall.MinCosineSimilarity`, calibrated per embedding model against +`gold-prod-2026-07`): a query embeds once per turn, FTS5 and vector top-k +candidates are unioned and fused, and any candidate below the floor is +dropped before ranking — zero survivors means zero injection. That floor is +real and measured (τ=0.67 for the current uint8-quantized embedding +variant), but a floor sweep across the gate-shootout's checksum run shows it +still injects *something* for the great majority of nothing-relevant queries: +**16.7% zero-injection accuracy** on `gold-prod-2026-07` (93 queries, in-sample +calibration set) and **7.3%** on a 450-query out-of-sample expansion +(`~/recall-research-local/2026-07/gold-expansion/`, disjoint from the +calibration set by normalized-text exclusion). The reason is structural, not +a mistuned constant: cosine similarity measures topical "aboutness" between a +query and a candidate, not "does this candidate help answer the question" — +a memory can be comfortably on-topic (cosine 0.74, well above a 0.67 floor) +and still be useless for the turn (e.g. an unrelated project fact that +happens to share vocabulary with the query). + +Four designs were measured head-to-head against this residual, then the +winner was re-validated out-of-sample on a gold set 4.8x larger than the one +used to pick it (`~/recall-research-local/2026-07/gate-shootout/` and +`gold-expansion/` respectively — both operator-local research stores holding +real, PII-bearing traffic; never committed, per the convention in +`docs/research/memory-audit-2026-07.md`). This design records that shoot-out, +the winning architecture, and the residuals that remain. + +**Actor/persistence context** (unchanged from memory-core-redesign): recall +runs on the session actor's turn path under `Memory.RecallTimeoutMs` (default +300 ms), executed by `SQLiteMemoryRecallCoordinator` +(`Netclaw.Actors/Sessions`). The embedding runtime lives behind the +consumer-defined `IMemoryEmbedder` seam (`Netclaw.Actors/Memory`), implemented +by `OnnxMemoryEmbedder` in `Netclaw.Embeddings`, resolved at call time through +the mutable `MemoryEmbedderHolder` (a plain DI singleton cannot hold a value +that is only known after `EmbeddingWarmupHostedService` finishes +provisioning, which necessarily runs after the DI container is built). +`EmbeddingModelProvisioner`'s pinned in-code allowlist (model id → URL, byte +size, SHA-256) is the supply-chain boundary: arbitrary URLs are never +accepted, only ids present in the allowlist. + +**Layering note**: this change's implementation targets the +`feature/memory-embeddings` branch, which carries memory-core-redesign's +embedding foundation, write-side nominate→decide, and read-side hybrid +recall slices ahead of `dev`. Because memory-core-redesign has not yet been +archived, the `memory-embeddings` capability does not yet exist under +`openspec/specs/`; this change's `specs/memory-embeddings/spec.md` delta is +therefore written against memory-core-redesign's own proposed spec +(`openspec/changes/memory-core-redesign/specs/memory-embeddings/spec.md`) as +its base, not against a synced main spec. If memory-core-redesign archives +(and syncs `memory-embeddings` into `openspec/specs/`) before this change +does, `opsx-sync` will need both deltas applied in dependency order — +memory-core-redesign's first, then this one. + +## Goals / Non-Goals + +**Goals** + +1. Close the measured residual: most nothing-relevant queries should inject + nothing, not "something topically adjacent." Target the validated + operating point (86.8% zero-injection accuracy out-of-sample), not just + the in-sample number. +2. Preserve recall: a query that has something genuinely relevant to say + should keep getting it. 98.3% recall retention out-of-sample is the + accepted cost, not zero cost — record this honestly. +3. Reuse memory-core-redesign's machinery wholesale — provisioning, + holder-and-warmup lifecycle, degradation contract, doctor/status surfaces + — so this change is a new manifest entry and a new scoring stage, not a + parallel subsystem. +4. Loud degradation: gate unavailability must never silently change recall + behavior without a marker. + +**Non-Goals** + +- Recalibrating the cosine floor, the fusion weights, or swapping the + embedding model — this change adds a stage strictly after that pipeline's + existing output. +- Domain-calibrated or class-conditional thresholds for the measured MS + MARCO under-scoring of procedural/command-style memories — recorded as a + residual, deferred. +- Re-running or expanding the judged gold sets further; the shoot-out and + gold-expansion results are consumed as already-ratified inputs. +- Ensembling multiple relevance models or scoring schemes. +- Collapsing `MemoryEmbedderHolder` and the new relevance-scorer holder into + a single combined holder — noted as an optional simplification (D4), not + required for this change. + +## Decisions + +### D1. Scorer seam + ONNX cross-encoder implementation, mirroring `IMemoryEmbedder` exactly + +`IRelevanceScorer` lives in `Netclaw.Actors/Memory` — a consumer-defined seam +in the same spirit as `IMemoryEmbedder`, so actor code never references +OnnxRuntime. Shape: + +- `string ModelId` — the allowlisted relevance-model id (vectors and scores + are never compared across models, same rule as embeddings). +- `bool IsAvailable` — real, expected false state (not provisioned, hash + failure, runtime load error); only calling the scoring method while + unavailable throws (matches `IMemoryEmbedder`'s contract exactly — no + garbage score silently corrupting the gate). +- `ValueTask> ScoreAsync(string query, IReadOnlyList candidates, CancellationToken ct)` + — batch, order-preserving, one call per turn for the ≤`AutoRecallMaxItems` + (3) floor survivors, mirroring `EmbedBatchAsync`'s batching rationale. + +`OnnxCrossEncoderScorer` (`Netclaw.Embeddings`) implements it: pair encoding +`[CLS] query [SEP] candidate [SEP]` with correct `token_type_ids` (0 for +query+CLS+SEP, 1 for candidate+final SEP), truncation strategy `only_second` +(caps the total at the model's max length by truncating only the candidate +side — a query is never truncated), dynamic sequence length bucketed to +multiples of 8 (the same bucketing convention `OnnxMemoryEmbedder` already +uses, avoiding a proliferation of ORT graph re-optimizations for arbitrary +lengths). The model's single `logits` output (shape `[batch,1]`) is passed +through a sigmoid host-side — the upstream model ships +`sbert_ce_default_activation_function: Identity`, so the activation is +explicitly not baked into the graph and must be applied by the caller. +`UnavailableRelevanceScorer` is the degraded-mode stub, matching +`UnavailableMemoryEmbedder`'s throw-on-call contract byte for byte. + +*Alternative considered*: extend `OnnxMemoryEmbedder`'s existing +`InferenceSession` to also serve cross-encoder inference — rejected: the +cross-encoder is a materially different model (a `BertForSequenceClassification` +pair-input head, not the bi-encoder's single-input pooling graph) with its +own tokenizer vocabulary; sharing a session would couple two independently +lifecycled models for no benefit. A second dedicated session, following the +exact same holder/warmup pattern, is simpler to reason about. + +### D2. Model selection: `Xenova/ms-marco-MiniLM-L-6-v2`, int8, chosen from a 4-design measured shoot-out + +| design | mechanism | in-sample verdict | out-of-sample verdict | +|---|---|---|---| +| A — distribution-shape | `z_top50 ≥ 2.80` (local-neighborhood outlier score) | 70.0% zero-inj, 100% retention — looked like a clean win | **Fails**: 65.2% zero-inj, **86.5% retention (below the ≥90% constraint)**, F0.5 0.089 < 0.100 floor-only | +| **B — cross-encoder (winner)** | `Xenova/ms-marco-MiniLM-L-6-v2`, pair scoring | 91.7% zero-inj (S*=0.08), 100% retention | **86.8% zero-inj (S*=0.02, 95% CI 82.3–90.3), 98.3% retention**, F0.5 0.130 vs 0.100 | +| C — learned feature gate | logistic/GBM over cosine/margin/z/length/age | candidate-level: 86.7% zero-inj across 10 CV folds (8 positive instances) | **Not viable**: query-level OOF AUC 0.545 (chance); candidate-level positives grew only 8→39 across the expansion, still insufficient, 80%-relative recall collapse on a differently-composed transfer set | +| D — per-memory offender priors | `pollution_count/injection_count` per docId | coverage ceiling measured directly, no separate OOS pass needed | **Structurally dead**: only 1.1% of top-3 candidates have 3+ injection history to build a prior from (5.3% even at a relaxed 2+ threshold); 80.6% of top-3 candidates are cold-start | + +Gate B is the only design whose out-of-sample result both replicates its +in-sample claim *and* clears the ≥90% recall-retention constraint. Its +in-sample recommended threshold (S*=0.08, chosen because gold-prod showed a +flat 100%-retention plateau from 0.02–0.08) turned out to be an artifact of +having only 8 floor-surviving true positives to calibrate against — the +450-query expansion grew that count to 39, and retention at S*=0.08 dropped +to 90.1% (exactly on the constraint boundary, zero margin). **The frozen +operating point for this change is S*=0.02**, which trades 1.8 points of +zero-injection accuracy (88.6%→86.8%) for 8.2 points of recall retention +(90.1%→98.3%) versus the in-sample-optimal S*=0.08 — the right side of that +trade given goal #2 above. + +Model artifact (frozen, quantized int8, the standard HuggingFace dynamic-INT8 +export — same family of artifact as the embedder's own quantization +options): + +- File: `model_quantized.onnx` +- Size: 23,143,499 bytes (22.07 MB) +- SHA-256: `e9d8ebf845c413e981c175bfe49a3bfa9b3dcce2a3ba54875ee5df5a58639fbe` + +The fp32 reference variant (`model.onnx`, 90,992,115 bytes / 86.78 MB, SHA-256 +`c623d0bcb99f4622beb413eaef00cfbe5db20df9f1dd982da4b4f26022881870`) was +measured bit-for-bit quality-identical to the quantized variant on both gold +sets and materially heavier on RAM (161–211 MB vs 48–103 MB incremental, +depending on measurement convention) for zero quality benefit — ruled out. + +*Alternative considered*: shipping Gate A (distribution-shape) as a cheap +first-pass filter ahead of Gate B — rejected: Gate A's out-of-sample failure +(recall retention below its own promised floor, F0.5 *worse* than doing +nothing) means it would need its own re-validation and threshold governance +for no measured benefit once Gate B is in place; not worth the added +moving part. + +### D3. Provisioning: a manifest *entry kind*, not a parallel allowlist + +The relevance model is provisioned through the exact same pinned-allowlist +mechanism `EmbeddingModelProvisioner` already implements for embedding +models — the allowlist gains a `RelevanceModelManifestEntry` alongside the +existing `EmbeddingModelManifestEntry`: `ModelId`, `ModelUrl`, `ModelSha256`, +`ModelByteSize`, and — the one field embedding manifests don't need — +`CalibratedThreshold` (S*=0.02). This is memory-core-redesign's +**manifest-carried operating point** pattern (the same zero-config mechanism +that let `MinCosineSimilarity` ship without requiring every operator to +calibrate their own floor): the threshold travels with the model id it was +measured against, so a future model swap cannot silently reuse a threshold +calibrated for a different model's score distribution. Download, atomic +temp+rename, and SHA-256 verification reuse the provisioner's existing code +path unchanged — this is a new manifest row and entry type, not new +download/verify logic. + +*Alternative considered*: a fully separate `RelevanceModelProvisioner` +class — rejected: the download/verify/reject-unknown-id logic has zero +model-kind-specific behavior; duplicating it would just be two copies of the +same supply-chain boundary to keep in sync. + +### D4. Warmup and holder: extend the existing warmup service; a third holder, not a forced merge + +`EmbeddingWarmupHostedService` gains a second provisioning step: when +`Memory.Embeddings.Enabled`, it provisions and warms the relevance model the +same way it does the embedding model (provision-or-degrade, one warm-up +inference call, gap-repair is not applicable here since there's no per-item +derived state to repair). The scorer is exposed through a new +`RelevanceScorerHolder`, following `MemoryEmbedderHolder`'s exact shape +(mutable holder, always non-null, initial value an `UnavailableRelevanceScorer` +stub, replaced once by the warmup service, read fresh on every use — never +cached by a consumer). + +Keeping three holders (`MemoryEmbedderHolder`, `MemoryVectorIndexHolder`, +`RelevanceScorerHolder`) rather than merging them keeps each concern +independently swappable and testable, consistent with what already exists. +**Consolidating the two model-runtime holders (embedder + relevance scorer) +into a single combined "embedding runtime holder"** is noted here as an +optional future simplification — both models are provisioned by the same +warmup step and share the same availability semantics, so a combined holder +would remove one moving part — but it is not required for this change and is +left as a follow-up decision rather than blocking this slice on a refactor +of already-shipped code. + +### D5. Recall wiring: a post-floor scoring stage under its own sub-budget + +In `SQLiteMemoryRecallCoordinator`, the gate applies strictly after the +existing hybrid-recall floor stage, and only in `hybrid` mode (a query +vector was available): the floor already reduced the candidate set to +`aboveFloor` (≤`AutoRecallMaxItems` = 3, per the shoot-out's exact +candidate-generation protocol — the gate never sees a candidate the floor +would not already have admitted). Each survivor is paired with the query and +scored via `RelevanceScorerHolder.Current.ScoreAsync`, under a CE sub-budget +(~60 ms) nested inside the overall `RecallTimeoutMs` via a linked +`CancellationTokenSource` — the same pattern the query-embedding sub-budget +already uses (measured p95 35 ms for 3 pairs leaves roughly 1.7x headroom +before the sub-budget itself is hit). Candidates scoring below the +manifest/config threshold are dropped; **zero survivors after the gate is a +"nothing injected" outcome**, identical in kind to zero survivors at the +floor — the `[memory-recall]` block continues to be omitted entirely, not +emitted empty. + +When the gate is unavailable, over its sub-budget, or recall is running in +`lexical` (degraded, no query vector) mode, the gate step is skipped +entirely and the floor's own output proceeds to injection unfiltered — this +is the same floor-only behavior that shipped in Slice 4, now reachable via +two independent degradation paths (embedder degraded → lexical mode already +skips the floor's cosine gate too; relevance-scorer degraded → floor's +cosine gate still applies, but no CE gate on top). + +*Alternative considered*: applying the gate to the full vector top-k (10 +candidates, before the floor) instead of just the ≤3 floor survivors — +rejected: this is exactly what the shoot-out measured and what the +out-of-sample validation certifies (candidates = floor-passing top-3); +scoring a wider candidate pool the gate was never validated against would +invalidate the calibrated threshold and roughly 3x the per-turn CE cost for +no measured benefit. + +### D6. Activation: one mental switch, explicit override only + +`Memory.Recall.RelevanceGate { Enabled, Threshold }`, both nullable: + +- `Enabled = null` (default) → follows `Memory.Embeddings.Enabled`. An + operator who turned on embeddings gets the gate; there is no second switch + to discover or forget to flip. +- `Enabled = true/false` → explicit override, independent of the embeddings + switch (e.g. an operator who wants embeddings for dedup/hybrid-recall but + not the extra CE latency per turn). +- `Threshold = null` (default) → follows the manifest's calibrated S* + (0.02) for whichever relevance model id is active. +- `Threshold = ` → explicit override, for an operator who re-runs the + shoot-out's threshold sweep against their own corpus and wants a different + operating point. + +This mirrors `MinCosineSimilarity`'s existing "config default, manifest +provides the calibrated number" relationship — no new configuration +philosophy, just one more nullable pair. + +### D7. Logging and eval coverage + +`memory_retrieval_final` gains two fields: `gateScores` (the CE score per +surviving-then-gated candidate, for post-hoc threshold analysis without +needing a fresh eval run) and `droppedByGate` (count, mirroring the existing +`filteredByFloor` field's shape). A new eval case seeds a corpus with +unrelated memories, asks an off-topic question, and asserts both that no +`[memory-recall]` block appears in the assembled prompt and that a gate +marker appears in the logs — the automated analogue of the shoot-out's +"zero-injection accuracy" metric, pinned as a regression gate rather than +left as a one-time measurement. + +### D8. Degradation semantics + +Model unavailable (not provisioned, hash failure, runtime load error) or CE +sub-budget exceeded ⇒ floor-only behavior (identical to pre-this-change +Slice 4 output) plus a rate-limited `memory_recall_gate_degraded` log +(matching the existing `memory_recall_vector_degraded` cooldown pattern — +loud on the first occurrence of a reason, not spammy on every subsequent +turn) and doctor visibility (extending the existing embedding doctor check +or adding a sibling relevance-gate doctor check — implementation detail for +tasks, not a design fork). The system never silently changes recall +selectivity without one of these signals firing. + +## Risks / Trade-offs + +- [MS MARCO domain mismatch under-scores procedural/command-style memories] + → measured, not hypothetical: of the 39 floor-surviving true positives in + the expanded gold set, 6 scored below S*=0.08 (2 below the frozen S*=0.02), + concentrated in release-workflow/procedural-context memories that are + useful-as-context but don't read as "the answer" to a cross-encoder trained + on MS MARCO's answer-passage judgments. At the frozen S*=0.02 this costs + ~1.7% of retained recall. Mitigation: recorded as a residual, not silently + absorbed; future work is domain calibration or a class-conditional + threshold for procedural/tool-lesson-adjacent memory classes. +- [Judge-agreement caveat on the validation set] → the 450-query expansion's + inter-rater agreement (κ=0.435, pooling 11 candidates/query, mostly + sub-floor and deliberately ambiguous) is materially below the original + July gold set's agreement (κ=0.754, judging only the 3 actually-injected + items/query — an easier, less skewed task). Mitigated by harsher-wins + aggregation (a doc counts as `relevant` only if both judging passes agreed) + which biases the expanded gold set conservative — the right bias for + validating a precision-oriented gate, but it means per-query labels in the + expansion are noisier than July's and the aggregate tables should be + trusted over any single query's label. +- [Threshold is model-conditional, like every other threshold in this + system] → S*=0.02 is calibrated specifically against + `Xenova/ms-marco-MiniLM-L-6-v2`'s score distribution; swapping the + relevance model without re-running the threshold sweep would silently + invalidate it. Mitigated the same way `MinCosineSimilarity` is: the + threshold travels in the manifest keyed to the model id (D3), not as a + bare config default disconnected from which model produced it. +- [Combined resource envelope is real but not free] → quantized CE adds + ~103 MB incremental RSS and ~11 ms p50 / ~35 ms p95 for 3 pairs on the + reference CPU; combined with int8 embeddings (263 MB) and daemon peak + (397 MB), the operator's measured total is ≈763 MB against a 1 GB K8s pod + limit — inside budget, but the margin (≈260 MB) is not so large that a + future addition to the memory runtime gets it for free. Mitigated by + measuring rather than assuming, and by keeping the CE sub-budget (~60 ms) + small relative to the overall 300 ms recall timeout so a degraded gate + never risks the turn itself. +- [Nested sub-budgets: query-embedding (~150 ms) + gate (~60 ms) inside one + 300 ms `RecallTimeoutMs`] → worst case both sub-budgets fully elapse + (210 ms) before any lexical/ranking work runs, leaving less slack than + Slice 4 alone had. Not yet measured end-to-end under production + contention. Flagged as an open question (below), not silently assumed + safe. +- [Two-holders-become-three] → `MemoryEmbedderHolder` + + `MemoryVectorIndexHolder` + the new `RelevanceScorerHolder` is more moving + parts than a consolidated holder would be. Accepted for this change (D4) + as consistent with the existing pattern; flagged as an optional future + consolidation rather than deferred silently. + +## Migration Plan + +1. Ships as an independent slice on top of `feature/memory-embeddings`'s + already-landed hybrid-recall stage (memory-core-redesign Slice 4). No + slice ordering dependency on any *other* part of memory-core-redesign + beyond what Slice 4 already requires. +2. Config-gated end to end: `Memory.Embeddings.Enabled = false` (the current + `dev` default) means the gate's provisioning step never runs and the + coordinator never attempts to resolve a `RelevanceScorerHolder` — zero + behavior change for any operator who hasn't already opted into + embeddings. `Memory.Recall.RelevanceGate.Enabled = false` is a second, + independent escape hatch for an operator who wants embeddings without the + gate's added per-turn latency. +3. Rollback: disabling either switch returns to exactly the prior Slice-4 + floor-only behavior; the relevance model artifact is derived/cacheable + data like the embedding model, safe to delete. +4. Schema: new `Memory.Recall.RelevanceGate` node added to + `netclaw-config.v1.schema.json`, all-nullable, migration-friendly per the + constitution's schema rules — no existing config document needs edits to + remain valid. +5. Calibration-verification harness: because the threshold is + model-conditional (Risk above), tasks include a short operator-facing note + (alongside the runbook, not a new production code path) describing how to + re-run the shoot-out's threshold-sweep protocol against a different + relevance model or a different corpus, so re-calibration is a documented + procedure rather than tribal knowledge trapped in a local research + directory. + +## Open Questions + +- Combined worst-case latency of the query-embedding sub-budget (~150 ms) + plus the new CE sub-budget (~60 ms) inside the single 300 ms + `RecallTimeoutMs`, measured end-to-end under realistic contention rather + than each sub-budget's own isolated measurement — gates this change's + sub-budget sizing the same way Slice 4 gated its own latency assumption + before shipping. +- Whether the deferred R2-mirroring decision for the embedding model artifact + (memory-core-redesign, post-PoC) should extend to this second (relevance) + model artifact once that decision is made. +- Whether to consolidate `MemoryEmbedderHolder` and `RelevanceScorerHolder` + into one combined embedding-runtime holder (D4) — left open rather than + decided, since both shapes are viable and the choice has no behavioral + consequence. diff --git a/openspec/changes/memory-relevance-gate/proposal.md b/openspec/changes/memory-relevance-gate/proposal.md new file mode 100644 index 000000000..f56cc536d --- /dev/null +++ b/openspec/changes/memory-relevance-gate/proposal.md @@ -0,0 +1,167 @@ +# Proposal: memory-relevance-gate + +Source PRD: PRD-007 (agent personality and local memory), continuing +`memory-core-redesign` (`openspec/changes/memory-core-redesign/`, PR #1570; +Slice 4 shipped the hybrid recall + calibrated cosine floor this change builds +on). Evidence base: `~/recall-research-local/2026-07/gate-shootout/` (4-design +gate shoot-out, 2026-07-06) and `~/recall-research-local/2026-07/gold-expansion/` +(450-query out-of-sample gold expansion + gate re-validation, 2026-07-06) — +operator-local research stores holding real (PII) traffic data, never +committed, per the same convention documented in +`docs/research/memory-audit-2026-07.md`. + +## Why + +Even with hybrid recall and the calibrated per-model cosine floor +(memory-core-redesign Slice 4), most nothing-relevant queries still cause an +injection: floor-only zero-injection accuracy measured **16.7%** on the July +gold set (`gold-prod-2026-07`, 93 queries) and **7.3%** on the 450-query +out-of-sample expanded gold set. Cosine similarity measures topical +"aboutness," not usefulness-for-answering — a candidate can clear the floor +and still be the wrong thing to inject. This is the dominant remaining +recall-quality defect because **60–65% of real queries have nothing relevant** +to recall at all (replicated across 543 labeled real-traffic queries: 93 July ++ 450 expansion), so the floor's residual miss rate lands on the majority +case, not the tail. + +## What Changes + +- **New relevance-gate stage after the cosine floor.** A tiny cross-encoder + scores `(query, candidate)` jointly for each of the (≤`AutoRecallMaxItems` + = 3) floor-surviving candidates; anything below a calibrated threshold S* is + dropped. Zero survivors after the gate ⇒ inject nothing, same as zero + survivors at the floor today. +- **Winner of a 4-design measured shoot-out, out-of-sample validated**: + `Xenova/ms-marco-MiniLM-L-6-v2`, `model_quantized.onnx` (int8, 22.07 MB, + SHA-256 `e9d8ebf845c413e981c175bfe49a3bfa9b3dcce2a3ba54875ee5df5a58639fbe`). + Out-of-sample (450-query expanded gold set, disjoint from the calibration + set) at S*=0.02: zero-injection accuracy **86.8%** (95% CI 82.3–90.3) vs + 7.3% floor-only, recall retention **98.3%**, F0.5 **0.130** vs 0.100 + floor-only, mean injected **0.251** vs 2.538 floor-only. +- **Reuses memory-core-redesign's infrastructure wholesale** — this is the + change's selling point, not an afterthought: the same consumer-defined-seam + pattern (`IMemoryEmbedder` → `IRelevanceScorer`), the same + allowlist-manifest provisioning pattern (`EmbeddingModelProvisioner` gains a + relevance-model manifest entry kind carrying pinned URL/SHA-256/size *and* + the calibrated operating threshold), the same warmup hosted service, and the + same loud-degradation contract (rate-limited log marker + doctor + visibility) — no new machinery class, only a new manifest entry and a new + scoring step in an existing pipeline. +- **One mental switch.** Gate activation is tied to + `Memory.Embeddings.Enabled` — there is no separate "turn semantic recall + quality on" knob. `Memory.Recall.RelevanceGate { Enabled (nullable, follows + Embeddings), Threshold (nullable, follows the manifest's calibrated S*) }` + exists only for an explicit operator override. +- **Logging.** `memory_retrieval_final` gains `gateScores` and `droppedByGate` + fields. A new eval case asserts the zero-injection behavior end-to-end: + seeded corpus, off-topic question, assert no `[memory-recall]` block and a + gate marker in the logs. +- **Rejected alternatives** (recorded for provenance; not shipped): + - *Distribution-shape statistical gate* (`z_top50 ≥ 2.80`): looked viable + in-sample (70% zero-injection) but failed out-of-sample — 65.2% + zero-injection accuracy, **86.5% recall retention (below the ≥90% + constraint)**, F0.5 0.089, *worse* than the 0.100 floor-only baseline. + - *Learned feature gate* (candidate-/query-level logistic regression and + GBM over cosine/margin/z-score/length/age features): query-level variant + measured out-of-fold AUC 0.545 (chance = 0.500, i.e. no signal); + candidate-level variant's positive-class support grew only 8→39 across + the gold expansion — still not enough to certify signal over + small-sample luck, and it showed an 80%-relative recall collapse on a + differently-composed transfer set. + - *Per-memory offender priors* (`pollution_count`/`injection_count` per + `docId`): structurally cold-start-bound — only 1.1% of top-3 recall + candidates carry 3+ injection observations to build a prior from, 5.3% + even at a relaxed 2+ threshold; 80.6% of top-3 candidates are cold-start + with no addressable history at all. + +## Capabilities + +### New Capabilities + +- `memory-relevance-gate`: the `IRelevanceScorer` seam and + `OnnxCrossEncoderScorer` implementation, the relevance-model provisioning + manifest kind (pinned URL/SHA-256/size + calibrated threshold), and the + post-floor gate stage wired into automatic recall. + +### Modified Capabilities + +- `netclaw-agent-memory`: the automatic pre-turn recall requirement gains a + post-floor relevance-gate stage — floor-surviving candidates are scored and + filtered before injection; zero survivors after the gate is a "nothing + injected" outcome exactly like zero survivors at the floor; gate + unavailability or sub-budget timeout degrades to floor-only behavior with a + loud marker. +- `memory-embeddings`: the pinned-allowlist provisioning requirement is + generalized to a manifest entry *kind* so it can provision relevance + (cross-encoder) models alongside embedding models, and the warmup hosted + service provisions/warms both. + +## Impact + +- **Code**: new `IRelevanceScorer` seam (`Netclaw.Actors/Memory`), new + `OnnxCrossEncoderScorer` (`Netclaw.Embeddings`, pair encoding `[CLS] q [SEP] + d [SEP]` with `token_type_ids`, sigmoid over the single-logit head, dynamic + sequence length bucket-of-8 matching the embedder's convention); + `EmbeddingModelProvisioner`'s allowlist gains a relevance-model manifest + kind; `SQLiteMemoryRecallCoordinator` gains the post-floor gate stage under + a CE sub-budget; `Netclaw.Configuration` gains + `Memory.Recall.RelevanceGate`; doctor/status surfaces extend to cover the + relevance model; `netclaw-memory` skill update. +- **Dependencies**: none new — reuses the `Microsoft.ML.OnnxRuntime` + + managed-tokenizer stack memory-core-redesign Slice 2 already adopted. One + new pinned model artifact (~22 MB int8), never embedded in the binary, + downloaded and hash-verified at provisioning time exactly like the + embedding model is today. +- **Data/config**: `netclaw-config.v1.schema.json` gains the new nodes, all + nullable with manifest-derived defaults — additive, non-breaking. +- **Evals**: new zero-injection gate eval case; `memory_retrieval_final`'s + log schema gains two additive fields (`gateScores`, `droppedByGate`). +- **Target branch**: implementation lands on `feature/memory-embeddings` (the + in-flight branch carrying memory-core-redesign's embedding and recall + slices), not directly on `dev` — this change's tasks assume that branch's + `IMemoryEmbedder`/`MemoryEmbedderHolder`/`SQLiteMemoryRecallCoordinator` + hybrid-recall code as their starting point. + +### In scope (MVP) + +- The cross-encoder scorer, its provisioning manifest entry, the coordinator + wiring (score → threshold → drop), the config surface, degradation + semantics, logging fields, and the zero-injection eval case. +- Recording the shoot-out's rejected alternatives and residual failure modes + in `design.md` for provenance. + +### Out of scope + +- Domain-calibrated or class-conditional thresholds for the measured MS + MARCO under-scoring of procedural/command-style memories (residual, ~1.7% + of retained recall at S*=0.02) — future work, not this change. +- Consolidating `MemoryEmbedderHolder` and a prospective relevance-scorer + holder into one combined embedding-runtime holder — noted as an optional + simplification in `design.md`, not required for this change to ship. +- Any change to the cosine floor itself, the embedding model, or the fusion + weights (memory-core-redesign Slice 4 territory; this change only adds a + stage after that pipeline's existing output). +- Re-running or expanding the judged gold sets further; this change consumes + the existing gate-shootout and gold-expansion results as already-ratified + inputs. + +## Security and Operational Impact + +- **Model supply chain**: the relevance model is provisioned through the + same pinned-allowlist mechanism as the embedding model — id → URL + byte + size + SHA-256, arbitrary URLs rejected, atomic download (temp + rename), + hash-verified before load. No new supply-chain surface, only a new + manifest entry kind on the existing one. +- **Resource envelope**: measured on the reference CPU — ~11 ms p50 / ~35 ms + p95 to score 3 pairs (quantized int8), ~103 MB incremental RSS. Combined + with int8 embeddings (263 MB) and daemon peak (397 MB), the operator's + measured total is ≈763 MB — inside the 1 GB K8s pod limit, with headroom + noted rather than assumed. +- **Degradation**: gate unavailability (model not provisioned) or exceeding + its CE sub-budget (~60 ms, linked CTS) degrades to floor-only behavior — the + pre-existing, already-shipped recall path — plus a rate-limited + `memory_recall_gate_degraded` log marker and doctor visibility. Never a + silent fallback, matching memory-core-redesign's degradation contract. +- **Operations**: no new operator action required — gate activation follows + `Memory.Embeddings.Enabled`; the existing warmup hosted service and doctor + checks extend to cover the new model without a new CLI verb. diff --git a/openspec/changes/memory-relevance-gate/specs/memory-embeddings/spec.md b/openspec/changes/memory-relevance-gate/specs/memory-embeddings/spec.md new file mode 100644 index 000000000..1e451615d --- /dev/null +++ b/openspec/changes/memory-relevance-gate/specs/memory-embeddings/spec.md @@ -0,0 +1,43 @@ +# Delta: memory-embeddings (memory-relevance-gate) + +## MODIFIED Requirements + +### Requirement: Pinned model provisioning + +Memory-subsystem models SHALL be selected by id from a pinned in-code +allowlist mapping model id to download URL, byte size, and SHA-256, covering +more than one kind of model artifact (embedding models and relevance-scoring +models share the same allowlist mechanism). A relevance-model manifest entry +SHALL additionally carry a calibrated similarity threshold alongside its +download and verification fields, so a model's operating point travels with +its id rather than living as a disconnected configuration default. Arbitrary +model URLs SHALL be rejected for every manifest kind. Provisioning SHALL +download atomically (temporary file then rename), verify the hash before +load, and run at daemon initialization when auto-download is enabled or on +explicit operator command. No model artifact SHALL be embedded in the +application binary. + +#### Scenario: Hash mismatch refuses the model + +- **GIVEN** a downloaded model artifact whose SHA-256 does not match the + allowlist entry +- **WHEN** provisioning verifies the artifact +- **THEN** the artifact is discarded and not loaded +- **AND** the failure is surfaced as a doctor-visible error + +#### Scenario: Unknown model id is rejected + +- **GIVEN** configuration naming a model id absent from the allowlist +- **WHEN** the daemon initializes embeddings +- **THEN** provisioning refuses with a configuration error identifying the + allowlisted ids + +#### Scenario: Relevance manifest entry's threshold travels with its model id + +- **GIVEN** an allowlisted relevance-model manifest entry carrying a + calibrated threshold +- **WHEN** that model id is provisioned and becomes active +- **THEN** the calibrated threshold from that same manifest entry is what + governs gating, not a threshold associated with any other model id +- **AND** switching to a different allowlisted relevance-model id switches + the effective threshold to that id's own calibrated value diff --git a/openspec/changes/memory-relevance-gate/specs/memory-relevance-gate/spec.md b/openspec/changes/memory-relevance-gate/specs/memory-relevance-gate/spec.md new file mode 100644 index 000000000..d0ed1bb26 --- /dev/null +++ b/openspec/changes/memory-relevance-gate/specs/memory-relevance-gate/spec.md @@ -0,0 +1,167 @@ +# Spec: memory-relevance-gate (new capability) + +## ADDED Requirements + +### Requirement: In-process cross-encoder relevance scoring + +The system SHALL score floor-surviving recall candidates against the query +with an in-process CPU ONNX cross-encoder — no sidecar processes, no network +inference hop, mirroring the embedding runtime's execution model. The scorer +SHALL sit behind a narrow interface owned by the memory subsystem so actor +code carries no ONNX dependency, SHALL preserve candidate order across a +batch call, and SHALL encode each `(query, candidate)` pair jointly (not as +two independently embedded vectors) so the score reflects usefulness for +answering the query rather than topical similarity alone. + +#### Scenario: Candidates score without external services + +- **GIVEN** a healthy daemon with the relevance model provisioned +- **WHEN** automatic recall has floor-surviving candidates to gate +- **THEN** each candidate is scored in-process against the query +- **AND** no network call or child process is involved in scoring + +#### Scenario: Query text is never truncated to fit a candidate + +- **GIVEN** a floor-surviving candidate whose combined length with the query + exceeds the model's maximum sequence length +- **WHEN** the pair is encoded for scoring +- **THEN** the candidate side is truncated to fit +- **AND** the query side is preserved in full + +### Requirement: Relevance model provisioning carries a calibrated operating point + +The relevance model SHALL be provisioned through the same pinned-allowlist +mechanism as other memory-subsystem models (id → download URL, byte size, +SHA-256, arbitrary URLs rejected), and its manifest entry SHALL additionally +carry a calibrated similarity threshold that travels with the model id. A +relevance model SHALL NOT be usable with a threshold calibrated for a +different model id. + +#### Scenario: Calibrated threshold ships with the model id + +- **GIVEN** the relevance model manifest entry for the active model id +- **WHEN** the recall coordinator applies the gate +- **THEN** it uses the threshold carried by that manifest entry unless the + operator has configured an explicit override +- **AND** no separate operator calibration step is required to get a + working default + +#### Scenario: Hash mismatch refuses the relevance model + +- **GIVEN** a downloaded relevance model artifact whose SHA-256 does not + match the allowlist entry +- **WHEN** provisioning verifies the artifact +- **THEN** the artifact is discarded and not loaded +- **AND** the gate reports unavailable rather than scoring with an unverified + artifact + +### Requirement: Post-floor relevance gate on automatic recall + +After the existing absolute cosine floor admits candidates, the system SHALL +score each surviving candidate (bounded to the automatic recall item limit) +against the query and SHALL drop any candidate whose score falls below the +active threshold. When every floor-surviving candidate is dropped by the +gate, the turn SHALL inject nothing, identical in kind to the existing +zero-survivors-at-the-floor outcome. The gate SHALL run under its own latency +sub-budget nested inside the overall recall timeout. + +#### Scenario: Topically-adjacent but unhelpful candidate is rejected + +- **GIVEN** a floor-surviving candidate whose cosine similarity to the query + clears the absolute floor but whose content does not help answer the query +- **WHEN** the relevance gate scores the candidate +- **THEN** the candidate scores below the active threshold +- **AND** the candidate is dropped before injection + +#### Scenario: Genuinely relevant candidate survives the gate + +- **GIVEN** a floor-surviving candidate that directly answers the query +- **WHEN** the relevance gate scores the candidate +- **THEN** the candidate scores above the active threshold +- **AND** the candidate remains eligible for injection + +#### Scenario: All candidates gated out means nothing injected + +- **GIVEN** every floor-surviving candidate for a turn scores below the + active threshold +- **WHEN** automatic recall completes for that turn +- **THEN** no memory items are injected +- **AND** the recall context block is omitted entirely from the prompt + +### Requirement: Loud degradation without silent fallback + +Automatic recall SHALL degrade to the floor-only result, unfiltered by the +relevance gate, when the relevance model is unavailable (not provisioned, +hash verification failed, runtime load error) or the gate exceeds its +per-turn sub-budget. The degraded state SHALL be loud: a doctor check +reports the cause, and a rate-limited structured log event records the +degradation reason. The system SHALL NOT silently apply or silently skip +gating without one of these signals. + +#### Scenario: Missing relevance model degrades to floor-only, loudly + +- **GIVEN** the relevance model is not provisioned +- **WHEN** a turn triggers automatic recall with floor-surviving candidates +- **THEN** recall injects the floor's own result unfiltered by any gate +- **AND** a rate-limited degradation event is logged +- **AND** `netclaw doctor` reports the missing relevance model with + remediation + +#### Scenario: Gate sub-budget timeout degrades to floor-only + +- **GIVEN** the relevance model is available but scoring exceeds its + configured sub-budget for a turn +- **WHEN** the sub-budget elapses +- **THEN** the gate stops waiting and recall injects the floor's own result + unfiltered for that turn +- **AND** the degradation is logged at a rate-limited interval, not on every + occurrence + +### Requirement: Gate activation follows embedding enablement + +The relevance gate SHALL be active whenever automatic embeddings are +enabled, without requiring a separate operator decision, while still +allowing an explicit override in either direction. The active similarity +threshold SHALL default to the value carried by the provisioned model's +manifest entry, while allowing an explicit operator override. + +#### Scenario: Enabling embeddings enables the gate with no extra configuration + +- **GIVEN** an operator enables automatic memory embeddings with no gate + configuration present +- **WHEN** the daemon starts +- **THEN** the relevance gate is active using the manifest-provided + threshold for the provisioned relevance model + +#### Scenario: Explicit override disables the gate independent of embeddings + +- **GIVEN** automatic memory embeddings are enabled +- **AND** the operator has explicitly disabled the relevance gate +- **WHEN** automatic recall runs +- **THEN** hybrid recall with the absolute cosine floor still applies +- **AND** no candidate is scored or dropped by the relevance gate + +### Requirement: Gate decisions are observable in retrieval logging and evals + +The final retrieval log record for a turn SHALL include the relevance score +computed for each gated candidate and the count of candidates dropped by the +gate. The eval suite SHALL include a case that seeds a memory corpus, poses +an off-topic query, and asserts both that no recall context block is added +to the prompt and that a gate-degradation-or-decision marker is present in +the logs for that turn. + +#### Scenario: Retrieval log records gate scores and drop count + +- **GIVEN** a turn where the relevance gate scored and dropped at least one + floor-surviving candidate +- **WHEN** the final retrieval log line is written +- **THEN** it includes the score computed for each gated candidate +- **AND** it includes the count of candidates the gate dropped + +#### Scenario: Zero-injection eval case passes on an off-topic query + +- **GIVEN** a seeded memory corpus with no content relevant to a specific + off-topic question +- **WHEN** the eval case asks that question +- **THEN** the assembled prompt contains no `[memory-recall]` block +- **AND** the turn's logs contain a relevance-gate marker for the decision diff --git a/openspec/changes/memory-relevance-gate/specs/netclaw-agent-memory/spec.md b/openspec/changes/memory-relevance-gate/specs/netclaw-agent-memory/spec.md new file mode 100644 index 000000000..91947f46b --- /dev/null +++ b/openspec/changes/memory-relevance-gate/specs/netclaw-agent-memory/spec.md @@ -0,0 +1,84 @@ +# Delta: netclaw-agent-memory (memory-relevance-gate) + +## MODIFIED Requirements + +### Requirement: Automatic pre-turn recall + +The system SHALL execute automatic recall before each user-facing model turn +using the latest user message, recent session context, active anchors, and +policy scope. Recall SHALL be hybrid: lexical (FTS5) and semantic (embedding +cosine) candidates are merged, and every candidate SHALL pass the identical +audience/boundary/sensitivity/recall-mode policy gates regardless of which +retriever surfaced it. Injection SHALL be gated by an absolute relevance +floor: when no candidate clears the configured minimum semantic similarity, +the turn SHALL inject nothing and the recall context block SHALL be omitted +entirely. Floor-surviving candidates SHALL additionally pass a relevance +gate — a cross-encoder scoring of each candidate jointly with the query — +before injection; when the gate is active and every floor-surviving +candidate scores below the active threshold, the turn SHALL inject nothing, +identical in kind to the floor's own zero-survivors outcome. Automatic +recall SHALL be bounded by a latency budget and SHALL degrade safely — to +lexical-only scoring with a structured degradation log when the embedder is +unavailable or over its sub-budget, to floor-only scoring with a structured +degradation log when the relevance gate is unavailable or over its +sub-budget, and to no injection when the memory substrate is unavailable. + +#### Scenario: Recall completes within budget + +- **GIVEN** the memory substrate is healthy +- **WHEN** a new turn begins +- **THEN** the session retrieves and injects a bounded recall bundle before the + model call +- **AND** the recall operation completes within the configured time budget or + degrades safely + +#### Scenario: Nothing relevant means nothing injected + +- **GIVEN** the memory store contains no memory semantically related to the + user's message +- **WHEN** automatic recall runs for the turn +- **THEN** no memory items are injected +- **AND** no recall context block is added to the prompt +- **AND** the retrieval log records zero injected items with the applied floor + +#### Scenario: Vector-sourced candidates obey policy gates + +- **GIVEN** a memory item excluded by the session's audience or sensitivity + policy +- **WHEN** the semantic retriever surfaces that item as a top cosine candidate +- **THEN** the item is filtered before scoring exactly as a lexical candidate + would be + +#### Scenario: Embedder degradation is loud, not silent + +- **GIVEN** the embedding runtime is unavailable or exceeds its per-turn + sub-budget +- **WHEN** automatic recall runs +- **THEN** recall proceeds lexical-only within the same latency budget +- **AND** a structured vector-degradation event is logged for diagnostics + +#### Scenario: Recall failure degrades without blocking the turn + +- **GIVEN** the memory database is temporarily unavailable +- **WHEN** the session starts automatic recall for a turn +- **THEN** the user-facing turn continues without durable recall injection +- **AND** the session records degraded memory status for diagnostics + +#### Scenario: Floor-surviving candidate that is not useful is gated out + +- **GIVEN** a candidate clears the absolute cosine floor but does not help + answer the user's message +- **WHEN** the relevance gate scores that candidate +- **THEN** the candidate is dropped before injection +- **AND** no recall context block is added for that candidate alone if it + was the only floor survivor + +#### Scenario: Relevance gate degradation is loud, not silent + +- **GIVEN** the relevance gate is unavailable or exceeds its per-turn + sub-budget +- **WHEN** automatic recall runs with candidates that survived the absolute + cosine floor +- **THEN** those candidates are injected unfiltered by the gate, within the + same latency budget +- **AND** a structured gate-degradation event is logged for diagnostics diff --git a/openspec/changes/memory-relevance-gate/tasks.md b/openspec/changes/memory-relevance-gate/tasks.md new file mode 100644 index 000000000..d2de8867e --- /dev/null +++ b/openspec/changes/memory-relevance-gate/tasks.md @@ -0,0 +1,77 @@ +# Tasks: memory-relevance-gate + +Implementation targets `feature/memory-embeddings` (memory-core-redesign +Slices 2–4 are this change's starting point, not `dev`). Slices are +independently shippable in order. + +## 1. Scorer, provisioning, manifest/config + +- [ ] 1.1 `IRelevanceScorer` seam in `Netclaw.Actors/Memory` (`ModelId`, + `IsAvailable`, order-preserving batch `ScoreAsync`) + + `UnavailableRelevanceScorer` stub, matching `IMemoryEmbedder`'s + throw-on-call-while-unavailable contract +- [ ] 1.2 `OnnxCrossEncoderScorer` in `Netclaw.Embeddings`: pair encoding + (`[CLS] query [SEP] candidate [SEP]`, correct `token_type_ids`, + `only_second` truncation so the query is never truncated), dynamic + sequence length bucketed to multiples of 8, sigmoid applied host-side + over the single-logit output +- [ ] 1.3 `RelevanceModelManifestEntry` (`ModelId`, `ModelUrl`, + `ModelSha256`, `ModelByteSize`, `CalibratedThreshold`) added to + `EmbeddingModelProvisioner`'s allowlist alongside the existing + embedding-model entries; pin `Xenova/ms-marco-MiniLM-L-6-v2` + `model_quantized.onnx` (22.07 MB, + SHA-256 `e9d8ebf845c413e981c175bfe49a3bfa9b3dcce2a3ba54875ee5df5a58639fbe`, + `CalibratedThreshold = 0.02`) +- [ ] 1.4 `RelevanceScorerHolder` (mirrors `MemoryEmbedderHolder`: mutable, + always non-null, initial `UnavailableRelevanceScorer`, replaced once by + the warmup service); `EmbeddingWarmupHostedService` gains a second + provision-or-degrade step (provision, hash-verify, one warm-up + inference) for the relevance model when `Memory.Embeddings.Enabled` +- [ ] 1.5 Config: `Memory.Recall.RelevanceGate { Enabled (nullable, follows + Embeddings.Enabled), Threshold (nullable, follows manifest + `CalibratedThreshold`) }` + `netclaw-config.v1.schema.json` sync with + defaults (additive, nullable, non-breaking) + +## 2. Coordinator wiring, degradation, tests, eval + +- [ ] 2.1 `SQLiteMemoryRecallCoordinator`: post-floor gate stage — score each + of the ≤`AutoRecallMaxItems` floor survivors under a ~60 ms CE + sub-budget (linked CTS nested inside `RecallTimeoutMs`, same pattern as + the existing query-embedding sub-budget); drop candidates below the + active threshold; zero survivors after the gate ⇒ inject nothing + (reuse the existing zero-injection path, don't fork it) +- [ ] 2.2 Degradation: relevance model unavailable, sub-budget exceeded, or + recall running in lexical (non-hybrid) mode ⇒ skip the gate entirely + and inject the floor's own result unfiltered; rate-limited + `memory_recall_gate_degraded` log (same cooldown pattern as + `memory_recall_vector_degraded`) +- [ ] 2.3 Doctor visibility for the relevance model (extend the existing + embedding doctor check or add a sibling check): model presence/hash, + provisioning failure, degraded-mode reason +- [ ] 2.4 Logging: `memory_retrieval_final` gains `gateScores` (per-candidate + score for every gated candidate) and `droppedByGate` (count) +- [ ] 2.5 Tests: pair-encoding correctness (token_type_ids, truncation-only- + second, dynamic length bucketing) against fixture pairs; threshold + admit/reject boundary; degraded-scorer fallback to floor-only; + sub-budget-timeout fallback; zero-survivors-after-gate produces the + same result shape as zero-survivors-at-the-floor; config + nullable-follows-manifest resolution (both `Enabled` and `Threshold`) +- [ ] 2.6 Eval case: seed a corpus with unrelated memories, ask an off-topic + question, assert no `[memory-recall]` block in the assembled prompt + and a gate marker present in the logs for that turn (the zero- + injection regression the gate exists to enforce) + +## 3. Docs, skill sync, scorecard, calibration note + +- [ ] 3.1 Update `netclaw-memory` skill: relevance gate exists, follows + `Memory.Embeddings.Enabled`, explicit override knobs, degraded-mode + behavior (floor-only fallback) +- [ ] 3.2 Runbook (`docs/runbooks/memory-health-and-evals.md`): relevance + gate section — doctor check, degradation log line, how to read + `gateScores`/`droppedByGate` in `memory_retrieval_final` +- [ ] 3.3 Record a scorecard in `design.md` (already drafted from the + shoot-out; keep in sync if any number changes before merge) and add a + short calibration-verification harness note (how to re-run the + threshold sweep against a different relevance model or corpus, so + re-calibration is a documented procedure, not tribal knowledge in a + local research directory) alongside the runbook diff --git a/src/Netclaw.Actors.Tests/Skills/SkillScannerTests.cs b/src/Netclaw.Actors.Tests/Skills/SkillScannerTests.cs index b6440bcf9..c287bbc8c 100644 --- a/src/Netclaw.Actors.Tests/Skills/SkillScannerTests.cs +++ b/src/Netclaw.Actors.Tests/Skills/SkillScannerTests.cs @@ -5,6 +5,7 @@ // ----------------------------------------------------------------------- using Netclaw.Actors.Skills; using Netclaw.Configuration; +using System.Linq; using Xunit; namespace Netclaw.Actors.Tests.Skills; @@ -813,4 +814,114 @@ private void WriteNestedSkill(string category, string skillName, string descript # {skillName} """); } + + [Fact] + public void ExtractFrontmatter_handles_utf8_bom() + { + // SKILL.md files saved by some editors (e.g., Notepad on Windows) include + // a UTF-8 BOM (\uFEFF) at the start of the file. ExtractFrontmatter should + // strip the BOM and still parse the frontmatter correctly. + var content = "\uFEFF---\nname: bom-skill\ndescription: \"A skill with BOM\"\n---\n\n# Content\n"; + + var result = SkillScanner.ExtractFrontmatter(content); + + Assert.NotNull(result); + Assert.Equal("bom-skill", result.Name); + Assert.Equal("A skill with BOM", result.Description); + } + + [Theory] + [InlineData("---\n---\n")] // empty frontmatter body + [InlineData("---\n---\n")] // BOM-prefixed empty frontmatter body + [InlineData("---\n---")] // no trailing newline + public void ExtractFrontmatter_returns_null_for_degenerate_block_without_throwing(string content) + { + // A degenerate block like "---\n---" has an empty YAML body: the opening line's + // newline IS the closing delimiter's newline. The slice must not compute a + // negative-length range (ArgumentOutOfRangeException) — it must return null so the + // file is reported as invalid frontmatter rather than crashing the scan. + var result = SkillScanner.ExtractFrontmatter(content); + + Assert.Null(result); + } + + [Fact] + public void Scan_does_not_abort_on_skill_with_degenerate_frontmatter() + { + // Regression: a SKILL.md whose frontmatter is an empty "---\n---" block previously + // threw ArgumentOutOfRangeException out of the unguarded parse call, aborting the + // entire discovery pass so that no skills loaded at all. Scan must instead skip the + // bad skill (recording an issue) and continue discovering healthy siblings. + WriteSkill("degenerate", "---\n---\n\n# Body\n"); + WriteSkill("healthy", """ + --- + name: healthy + description: A perfectly good skill. + --- + + # Healthy + """); + + var result = SkillScanner.Scan(_skillsDir); + + Assert.Contains(result.AcceptedSkills, s => s.Name == "healthy"); + Assert.Contains(result.Issues, i => + i.Path.EndsWith(Path.Combine("degenerate", "SKILL.md"), StringComparison.OrdinalIgnoreCase)); + } + + [Fact] + public void SkillScanIssue_populates_skill_name_for_broken_frontmatter() + { + // When a SKILL.md has invalid frontmatter, the resulting SkillScanIssue + // should include the SkillName (derived from the parent directory name) + // so that issue reporting can identify the skill by name. + WriteSkill("broken-frontmatter", """ + --- + name: broken-frontmatter + description: [invalid yaml {{{ + --- + + # Broken + """); + + var result = SkillScanner.Scan(_skillsDir); + + Assert.Empty(result.AcceptedSkills); // broken frontmatter => skill rejected + var issuesForSkill = result.Issues + .Where(i => i.Path.EndsWith(Path.Combine("broken-frontmatter", "SKILL.md"), StringComparison.OrdinalIgnoreCase)) + .ToList(); + + Assert.NotEmpty(issuesForSkill); + Assert.All(issuesForSkill, i => + { + Assert.NotNull(i.SkillName); + Assert.Equal("broken-frontmatter", i.SkillName); + }); + } + + [Fact] + public void SkillScanIssue_normalizes_skill_name_from_mixed_case_directory() + { + // Issue SkillNames must be the canonical (lowercased) skill name — the same + // representation accepted skills use — so that errored and accepted rows render + // consistently regardless of the on-disk directory casing. + WriteSkill("Mixed-Case", """ + --- + name: Mixed-Case + description: [invalid yaml {{{ + --- + + # Broken + """); + + var result = SkillScanner.Scan(_skillsDir); + + var issuesForSkill = result.Issues + .Where(i => i.Path.EndsWith(Path.Combine("Mixed-Case", "SKILL.md"), StringComparison.OrdinalIgnoreCase)) + .ToList(); + + Assert.NotEmpty(issuesForSkill); + Assert.All(issuesForSkill, i => Assert.Equal("mixed-case", i.SkillName)); + } + } diff --git a/src/Netclaw.Actors.Tests/SubAgents/RecordingSessionMetrics.cs b/src/Netclaw.Actors.Tests/SubAgents/RecordingSessionMetrics.cs new file mode 100644 index 000000000..8f2ff1c56 --- /dev/null +++ b/src/Netclaw.Actors.Tests/SubAgents/RecordingSessionMetrics.cs @@ -0,0 +1,49 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using Netclaw.Actors.Telemetry; +using Netclaw.Configuration; + +namespace Netclaw.Actors.Tests.SubAgents; + +/// +/// Records every call so a test can +/// assert a sub-agent bills each LLM call to the daily-stats sink. Thread-safe: the +/// actor records on its mailbox thread while the test reads after the Ask +/// completes. Regression support for issue #1597. +/// +internal sealed class RecordingSessionMetrics : ISessionMetrics +{ + private readonly object _gate = new(); + private readonly List<(long Input, long Output)> _tokenUsageCalls = []; + private long _totalInput; + private long _totalOutput; + + public IReadOnlyList<(long Input, long Output)> TokenUsageCalls + { + get { lock (_gate) { return _tokenUsageCalls.ToArray(); } } + } + + public long TotalInputTokens { get { lock (_gate) { return _totalInput; } } } + + public long TotalOutputTokens { get { lock (_gate) { return _totalOutput; } } } + + public void RecordTokenUsage(long inputTokens, long outputTokens) + { + lock (_gate) + { + _tokenUsageCalls.Add((inputTokens, outputTokens)); + _totalInput += inputTokens; + _totalOutput += outputTokens; + } + } + + public void RecordTurnCompleted() { } + public void RecordSessionCreated() { } + public void RecordMemoriesFormed(int count) { } + public void RecordMemoriesRecalled(int count) { } + public void RecordSkillsLoaded(int count) { } + public void RecordSkillLoaded(string skillName, SkillLoadMethod method) { } +} diff --git a/src/Netclaw.Actors.Tests/SubAgents/SubAgentActorTests.cs b/src/Netclaw.Actors.Tests/SubAgents/SubAgentActorTests.cs index 7b1f58b55..0a6ffd8f2 100644 --- a/src/Netclaw.Actors.Tests/SubAgents/SubAgentActorTests.cs +++ b/src/Netclaw.Actors.Tests/SubAgents/SubAgentActorTests.cs @@ -1415,6 +1415,14 @@ internal sealed class FakeChatClient : IChatClient public IReadOnlyList? ResponseTextsByCall { get; set; } + /// + /// When set, every returned response carries these token counts as + /// . The streaming reader coalesces that back into + /// response.Usage, so a test can prove the sub-agent bills each LLM call's + /// tokens to . + /// + public UsageDetails? UsageOverride { get; set; } + public async Task GetResponseAsync( IEnumerable messages, ChatOptions? options = null, @@ -1437,7 +1445,7 @@ public async Task GetResponseAsync( var toolCallContents = new List(ToolCallsOnFirstCall); var toolCallMessage = new ChatMessage( ChatRole.Assistant, toolCallContents); - return new ChatResponse(toolCallMessage); + return new ChatResponse(toolCallMessage) { Usage = UsageOverride }; } } @@ -1448,7 +1456,7 @@ public async Task GetResponseAsync( var responseMessage = new ChatMessage( ChatRole.Assistant, [new TextContent(responseText)]); - return new ChatResponse(responseMessage); + return new ChatResponse(responseMessage) { Usage = UsageOverride }; } public IAsyncEnumerable GetStreamingResponseAsync( diff --git a/src/Netclaw.Actors.Tests/SubAgents/SubAgentObservabilityTests.cs b/src/Netclaw.Actors.Tests/SubAgents/SubAgentObservabilityTests.cs index c93cc824e..c37161dfc 100644 --- a/src/Netclaw.Actors.Tests/SubAgents/SubAgentObservabilityTests.cs +++ b/src/Netclaw.Actors.Tests/SubAgents/SubAgentObservabilityTests.cs @@ -107,4 +107,79 @@ await agent.Ask( NewRun("Greet the user"), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); }, cancellationToken: TestContext.Current.CancellationToken); } + + // Regression coverage for issue #1597: sub-agent LLM calls used to discard + // ChatResponse.Usage entirely — the actor had no ISessionMetrics and never read + // response.Usage — so every sub-agent's token consumption was invisible to + // `netclaw stats`. These tests pin the sub-agent to the shared daily-stats sink. + + [Fact] + public async Task Records_token_usage_to_session_metrics_on_text_response() + { + var metrics = new RecordingSessionMetrics(); + var fakeClient = new FakeChatClient + { + UsageOverride = new UsageDetails { InputTokenCount = 120, OutputTokenCount = 45 } + }; + var agent = Sys.ActorOf(SubAgentActor.CreateProps( + CreateDefinition(), fakeClient, sessionMetrics: metrics)); + + var result = await agent.Ask( + NewRun("Say hello"), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); + + Assert.True(result.Success); + // A single LLM call → exactly one usage record billed to the shared + // process-wide daily-stats sink (the same singleton the parent session uses). + var call = Assert.Single(metrics.TokenUsageCalls); + Assert.Equal((120L, 45L), call); + } + + [Fact] + public async Task Records_token_usage_for_every_llm_call_across_the_turn_loop() + { + // A tool-call turn followed by a final-text turn = two LLM calls. Both must be + // billed. This is the crux of #1597: the sub-agent's INTERNAL calls (not just + // its single final output) have to reach `netclaw stats`, so the recorded total + // is the per-call usage summed — not one call's worth. + var metrics = new RecordingSessionMetrics(); + var fakeTool = new FakeNetclawTool("greet", "Hello from tool!"); + var fakeClient = new FakeChatClient + { + ToolCallsOnFirstCall = + [ + new FunctionCallContent("call-1", "greet", + new Dictionary { ["name"] = "World" }) + ], + UsageOverride = new UsageDetails { InputTokenCount = 120, OutputTokenCount = 45 } + }; + var agent = Sys.ActorOf(SubAgentActor.CreateProps( + CreateDefinition([fakeTool]), fakeClient, sessionMetrics: metrics)); + + var result = await agent.Ask( + NewRun("Greet the user"), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); + + Assert.True(result.Success); + Assert.Equal(2, fakeClient.CallCount); + Assert.Equal(2, metrics.TokenUsageCalls.Count); + Assert.Equal(240L, metrics.TotalInputTokens); + Assert.Equal(90L, metrics.TotalOutputTokens); + } + + [Fact] + public async Task Completion_summary_reports_cumulative_token_totals() + { + var fakeClient = new FakeChatClient + { + UsageOverride = new UsageDetails { InputTokenCount = 120, OutputTokenCount = 45 } + }; + var agent = Sys.ActorOf(SubAgentActor.CreateProps(CreateDefinition(), fakeClient)); + + // The completion summary now carries token totals so sub-agent cost is visible + // in the logs (and Seq), not just tool/iteration/duration counts. + await EventFilter.Info(contains: "inputTokens=120, outputTokens=45").ExpectAsync(1, async () => + { + await agent.Ask( + NewRun("Say hello"), TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); + }, cancellationToken: TestContext.Current.CancellationToken); + } } diff --git a/src/Netclaw.Actors.Tests/SubAgents/SubAgentSpawnerTests.cs b/src/Netclaw.Actors.Tests/SubAgents/SubAgentSpawnerTests.cs index 21293c31c..cca1518b6 100644 --- a/src/Netclaw.Actors.Tests/SubAgents/SubAgentSpawnerTests.cs +++ b/src/Netclaw.Actors.Tests/SubAgents/SubAgentSpawnerTests.cs @@ -150,6 +150,69 @@ public async Task Spawn_async_ignores_definition_tool_metadata_for_runtime_tool_ Assert.Equal(1, started.ToolCount); } + [Fact] + public async Task Spawned_sub_agent_bills_its_llm_calls_to_session_metrics() + { + // Full-wiring regression guard for #1597: a sub-agent spawned through the real + // SubAgentSpawner must record its LLM-call tokens to the ISessionMetrics handed + // to the spawner. Unlike the actor-level tests, this exercises the + // spawner -> CreateProps -> actor pass-through, so dropping the metrics argument + // anywhere along that chain fails here. The SpawnChildActor factory materializes + // the spawner-built Props into a real SubAgentActor (a probe stand-in would + // bypass CreateProps entirely and hide a broken pass-through). + var toolRegistry = new ToolRegistry(); + toolRegistry.Register(new FakeNetclawTool("inspect_context", "ok")); + + var metrics = new RecordingSessionMetrics(); + var chatClient = new FakeChatClient + { + UsageOverride = new UsageDetails { InputTokenCount = 175, OutputTokenCount = 60 } + }; + + var spawner = new SubAgentSpawner( + new SingleClientProvider(chatClient), + toolRegistry, + new ToolAccessPolicy( + new ToolConfig(), + new EffectivePolicyDefaults( + DeploymentPosture.Personal, + TrustAudience.Personal, + ShellExecutionMode.HostAllowed, + UsedStrictFallback: false), + new ShellCommandPolicy()), + approvalService: null, + new StaticSystemPromptProvider("You are a summarizer."), + NullLogger.Instance, + sessionMetrics: metrics); + + var context = new ToolExecutionContext("console/subagent-parent", "/tmp/netclaw/sessions/parent") + { + Audience = TrustAudience.Personal + }; + context.SpawnChildActor = (props, name, _) => Task.FromResult(Sys.ActorOf((Props)props, name)); + + var profile = new SubAgentProfile + { + Name = "summarizer", + Description = "Summarize content", + SystemPrompt = "You are a summarizer.", + ToolNames = ["inspect_context"], + Visibility = SubAgentVisibility.UserFacing + }; + + var result = await spawner.SpawnAsync( + profile, + "Summarize the repo.", + runtimeContext: null, + context, + TestContext.Current.CancellationToken); + + Assert.True(result.Success, $"Expected success but got: {result.Output}"); + // One text-only LLM call → exactly one usage record, carrying the fake's tokens. + var call = Assert.Single(metrics.TokenUsageCalls); + Assert.Equal((175L, 60L), call); + } + private sealed class NoOpChatClient : IChatClient { public Task GetResponseAsync( diff --git a/src/Netclaw.Actors/Skills/SkillScanner.cs b/src/Netclaw.Actors/Skills/SkillScanner.cs index 0b78b456e..3fa5fc3fd 100644 --- a/src/Netclaw.Actors/Skills/SkillScanner.cs +++ b/src/Netclaw.Actors/Skills/SkillScanner.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // // Copyright (C) 2026 - 2026 Petabridge, LLC // @@ -280,24 +280,31 @@ private static void MergeSources( } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { + var skillName = NormalizeSkillName(Path.GetFileName(Path.GetDirectoryName(canonicalSkillFilePath)!)); issues.Add(new SkillScanIssue( Path: canonicalSkillFilePath, Kind: SkillScanIssueKind.UnreadableFile, - Message: $"Failed to read skill file: {ex.Message}")); + Message: $"Failed to read skill file: {ex.Message}", + SkillName: skillName)); return null; } var frontmatter = ExtractFrontmatter(content); if (frontmatter is null) { + var skillName = NormalizeSkillName(Path.GetFileName(Path.GetDirectoryName(canonicalSkillFilePath)!)); + // content is from File.ReadAllText, which already strips any UTF-8 BOM, so no TrimStart + // is needed here; BOM tolerance for direct-string callers lives in ExtractFrontmatter. + var hasFrontmatterStart = content.StartsWith("---", StringComparison.Ordinal); issues.Add(new SkillScanIssue( Path: canonicalSkillFilePath, - Kind: content.StartsWith("---", StringComparison.Ordinal) + Kind: hasFrontmatterStart ? SkillScanIssueKind.InvalidFrontmatter : SkillScanIssueKind.MissingFrontmatter, - Message: content.StartsWith("---", StringComparison.Ordinal) + Message: hasFrontmatterStart ? "Skill frontmatter is invalid or unparseable." - : "Skill file must start with YAML frontmatter.")); + : "Skill file must start with YAML frontmatter.", + SkillName: skillName)); return null; } @@ -329,41 +336,47 @@ private static void MergeSources( } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { + var skillName = NormalizeSkillName(Path.GetFileNameWithoutExtension(canonicalPath)); issues.Add(new SkillScanIssue( Path: canonicalPath, Kind: SkillScanIssueKind.UnreadableFile, - Message: $"Failed to read flat skill file: {ex.Message}")); + Message: $"Failed to read flat skill file: {ex.Message}", + SkillName: skillName)); return null; } var frontmatter = ExtractFrontmatter(content); if (frontmatter is null) { + // content is from File.ReadAllText (BOM already stripped), so no TrimStart needed. if (allowFrontmatterlessFlatFiles && !content.StartsWith("---", StringComparison.Ordinal)) return BuildFlatSkillEntryWithoutFrontmatter(canonicalPath, canonicalRoot, content, issues); + var skillName = NormalizeSkillName(Path.GetFileNameWithoutExtension(canonicalPath)); issues.Add(new SkillScanIssue( Path: canonicalPath, Kind: SkillScanIssueKind.FlatFileMissingFrontmatter, - Message: "Flat .md file found but lacks valid YAML frontmatter. Add frontmatter with name and description, or move into a skill-name/SKILL.md directory.")); + Message: "Flat .md file found but lacks valid YAML frontmatter. Add frontmatter with name and description, or move into a skill-name/SKILL.md directory.", + SkillName: skillName)); return null; } + // Derive skill name from frontmatter or filename + var fileNameWithoutExt = Path.GetFileNameWithoutExtension(canonicalPath); + var name = !string.IsNullOrWhiteSpace(frontmatter.Name) + ? NormalizeSkillName(frontmatter.Name) + : NormalizeSkillName(fileNameWithoutExt); + if (string.IsNullOrWhiteSpace(frontmatter.Description)) { issues.Add(new SkillScanIssue( Path: canonicalPath, Kind: SkillScanIssueKind.FlatFileNoDescription, - Message: "Flat .md file has frontmatter but missing description field.")); + Message: "Flat .md file has frontmatter but missing description field.", + SkillName: name)); return null; } - // Derive skill name from frontmatter or filename - var fileNameWithoutExt = Path.GetFileNameWithoutExtension(canonicalPath); - var name = !string.IsNullOrWhiteSpace(frontmatter.Name) - ? NormalizeSkillName(frontmatter.Name) - : NormalizeSkillName(fileNameWithoutExt); - if (strictNameMatch && !string.IsNullOrWhiteSpace(frontmatter.Name)) { var expectedName = NormalizeSkillName(fileNameWithoutExt); @@ -405,6 +418,8 @@ private static void MergeSources( /// public static SkillFrontmatter? ExtractFrontmatter(string content) { + // Strip UTF-8 BOM — some editors (e.g., Notepad on Windows) prepend \uFEFF + content = content.TrimStart('\uFEFF'); if (!content.StartsWith("---", StringComparison.Ordinal)) return null; @@ -413,7 +428,16 @@ private static void MergeSources( if (closingIndex < 0) return null; - var yamlBlock = content[(content.IndexOf('\n', 0) + 1)..closingIndex]; + // Guard degenerate blocks like "---\n---" where the opening line's newline IS the + // closing delimiter: the YAML body is empty, so there is nothing to deserialize. + // Without this, content[(firstNewline+1)..closingIndex] slices a negative-length + // range and throws ArgumentOutOfRangeException, which propagates out of Scan (the + // parse calls are unguarded) and aborts the entire skill-discovery pass. + var firstNewline = content.IndexOf('\n', StringComparison.Ordinal); + if (firstNewline < 0 || firstNewline >= closingIndex) + return null; + + var yamlBlock = content[(firstNewline + 1)..closingIndex]; try { @@ -451,10 +475,14 @@ internal static string ExtractBody(string content) // Description is required per AgentSkills.io spec if (string.IsNullOrWhiteSpace(fm.Description)) { + var skillName = !string.IsNullOrWhiteSpace(fm.Name) + ? NormalizeSkillName(fm.Name) + : NormalizeSkillName(Path.GetFileName(skillDirectory)); issues.Add(new SkillScanIssue( Path: filePath, Kind: SkillScanIssueKind.MissingDescription, - Message: "Skill frontmatter must include a non-empty description.")); + Message: "Skill frontmatter must include a non-empty description.", + SkillName: skillName)); return null; } @@ -581,10 +609,14 @@ private static (bool HasSubagentMetadata, string? Subagent, string? Error) Parse } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { + // skillDirectory is already the skill's own directory, so its leaf name IS the + // skill name — do not climb to the parent (that yields the container dir, e.g. "files"). + var skillName = NormalizeSkillName(Path.GetFileName(skillDirectory)); issues.Add(new SkillScanIssue( Path: skillDirectory, Kind: SkillScanIssueKind.ResourceEnumerationFailed, - Message: $"Failed to enumerate resources: {ex.Message}")); + Message: $"Failed to enumerate resources: {ex.Message}", + SkillName: skillName)); return null; } @@ -632,10 +664,12 @@ private static string Truncate(string value, int maxLength) var description = ExtractFirstNonEmptyMarkdownLine(content); if (string.IsNullOrWhiteSpace(description)) { + var skillName = NormalizeSkillName(Path.GetFileNameWithoutExtension(canonicalPath)); issues.Add(new SkillScanIssue( Path: canonicalPath, Kind: SkillScanIssueKind.FlatFileNoDescription, - Message: "Flat .md file without frontmatter must contain at least one non-empty line to infer a description.")); + Message: "Flat .md file without frontmatter must contain at least one non-empty line to infer a description.", + SkillName: skillName)); return null; } diff --git a/src/Netclaw.Actors/SubAgents/SubAgentActor.cs b/src/Netclaw.Actors/SubAgents/SubAgentActor.cs index d28f62512..7dde15929 100644 --- a/src/Netclaw.Actors/SubAgents/SubAgentActor.cs +++ b/src/Netclaw.Actors/SubAgents/SubAgentActor.cs @@ -63,6 +63,13 @@ [Subagent Execution Contract] private readonly ToolAccessPolicy _toolAccessPolicy; private readonly IToolApprovalService? _approvalService; private readonly int _maxToolIterations; + + // Process-wide daily-stats sink (the same singleton the parent session records + // to). Nullable because a hosting configuration without the daemon stats backend + // is a real runtime state — mirrors LlmSessionActor._sessionMetrics. When present, + // every LLM call this sub-agent makes is billed here so its tokens show up in + // `netclaw stats` instead of vanishing. + private readonly Telemetry.ISessionMetrics? _sessionMetrics; private readonly ToolRegistry _toolRegistry; private IReadOnlyList _aiTools = []; private ILoggingAdapter _log; @@ -74,6 +81,12 @@ [Subagent Execution Contract] // timer scheduler — it doesn't track elapsed time itself). private readonly Stopwatch _runStopwatch = Stopwatch.StartNew(); + // Cumulative token usage across every LLM call this sub-agent makes. Summed for + // the completion summary log; per-call usage is also recorded to _sessionMetrics + // as each call returns (see RecordUsage). + private long _runInputTokens; + private long _runOutputTokens; + // Conversation state (not persisted — ephemeral) private readonly List _history = []; private long _llmCallId; @@ -137,7 +150,8 @@ public SubAgentActor( IChatClient chatClient, ToolAccessPolicy? toolAccessPolicy = null, IToolApprovalService? approvalService = null, - int maxToolIterations = DefaultMaxToolIterations) + int maxToolIterations = DefaultMaxToolIterations, + Telemetry.ISessionMetrics? sessionMetrics = null) { if (maxToolIterations <= 0) throw new ArgumentOutOfRangeException(nameof(maxToolIterations), maxToolIterations, @@ -145,6 +159,7 @@ public SubAgentActor( _definition = definition; _chatClient = chatClient; + _sessionMetrics = sessionMetrics; _toolAccessPolicy = toolAccessPolicy ?? new ToolAccessPolicy( new ToolConfig { ShellMode = ShellExecutionMode.HostAllowed }, new EffectivePolicyDefaults( @@ -175,14 +190,16 @@ public static Props CreateProps( IChatClient chatClient, ToolAccessPolicy? toolAccessPolicy = null, IToolApprovalService? approvalService = null, - int maxToolIterations = DefaultMaxToolIterations) + int maxToolIterations = DefaultMaxToolIterations, + Telemetry.ISessionMetrics? sessionMetrics = null) { return Props.Create(() => new SubAgentActor( definition, chatClient, toolAccessPolicy, approvalService, - maxToolIterations)); + maxToolIterations, + sessionMetrics)); } /// @@ -320,6 +337,14 @@ private void Processing() // the synchronous processing that follows (tool dispatch or completion). RestartWatchdog(_interDeltaBudget); var response = msg.Response; + + // Record this call's token usage before branching so EVERY call is billed — + // tool-call turns, retries, the forced-no-tools final turn, and repair turns + // all flow through here exactly once. Mirrors the main session, which records + // its own per-call usage; without this the sub-agent's tokens never reach the + // daily-stats pipeline and `netclaw stats` under-counts by the whole sub-run. + RecordUsage(response.Usage); + var lastMessage = response.Messages[^1]; var analysis = LlmResponseClassifier.Analyze(lastMessage); @@ -630,6 +655,24 @@ private void Processing() }); } + // Bill one LLM call's token usage to the shared daily-stats sink and accumulate + // the run totals for the completion summary log. We record at the source (here in + // the child) rather than propagating totals up to the parent: the parent's + // ISessionMetrics is the SAME process-wide singleton, so re-recording there would + // double-count, and folding sub-agent tokens into the parent's UsageOutput would + // corrupt its context-window percentage (the sub-agent has its own context window). + private void RecordUsage(UsageDetails? usage) + { + if (usage is null) + return; + + var input = usage.InputTokenCount ?? 0; + var output = usage.OutputTokenCount ?? 0; + _runInputTokens += input; + _runOutputTokens += output; + _sessionMetrics?.RecordTokenUsage(input, output); + } + private void HandleToolCalls(AiChatMessage assistantMessage, List toolCalls) { _turnState.ResetEmptyResponseGuards(); @@ -752,13 +795,17 @@ private void Complete( _log.Info("SubAgent [{AgentName}] completed (success={Success}, outcome={Outcome}, reason={Reason}, output={OutputLength} chars, iterations={Iterations})", _definition.Name, success, resolvedOutcome, outcomeReason?.Value ?? "-", output.Length, _turnState.ToolIterationCount); - // Log cumulative stats for observability — total LLM calls, tool usage, etc. - // This gives operators a single summary line for sub-agent duration analysis. + // Log cumulative stats for observability — total LLM calls, tool usage, tokens. + // This gives operators a single summary line for sub-agent cost/duration analysis. + // (success is already on the "completed" line above; omitted here to stay within + // ILoggingAdapter's 6-argument ceiling.) _log.Info( - "SubAgent [{AgentName}] summary: success={Success}, totalToolCalls={TotalToolCalls}, " - + "iterations={Iterations}, duration={Duration}s", - _definition.Name, success, _turnState.ToolCallCount, + "SubAgent [{AgentName}] summary: totalToolCalls={TotalToolCalls}, " + + "iterations={Iterations}, inputTokens={InputTokens}, outputTokens={OutputTokens}, " + + "duration={Duration}s", + _definition.Name, _turnState.ToolCallCount, _turnState.ToolIterationCount, + _runInputTokens, _runOutputTokens, _runStopwatch.Elapsed.TotalSeconds); var findings = success && _definition.EmitStructuredFindings diff --git a/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs b/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs index de2b673ea..530cda8b1 100644 --- a/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs +++ b/src/Netclaw.Actors/SubAgents/SubAgentSpawner.cs @@ -33,6 +33,12 @@ public sealed class SubAgentSpawner private readonly SubAgentConfig _subAgentConfig; private readonly ILogger _logger; + // The process-wide daily-stats sink, handed to each spawned SubAgentActor so its + // LLM calls are billed to `netclaw stats`. Nullable to match the rest of the stats + // wiring (a host without the daemon stats backend is a real runtime state); DI + // injects the registered singleton in production. + private readonly Telemetry.ISessionMetrics? _sessionMetrics; + public SubAgentSpawner( IChatClientProvider chatClientProvider, ToolRegistry toolRegistry, @@ -40,7 +46,8 @@ public SubAgentSpawner( IToolApprovalService? approvalService, ISystemPromptProvider promptProvider, ILogger logger, - SubAgentConfig? subAgentConfig = null) + SubAgentConfig? subAgentConfig = null, + Telemetry.ISessionMetrics? sessionMetrics = null) { _chatClientProvider = chatClientProvider; _toolRegistry = toolRegistry; @@ -49,6 +56,7 @@ public SubAgentSpawner( _promptProvider = promptProvider; _subAgentConfig = subAgentConfig ?? new SubAgentConfig(); _logger = logger; + _sessionMetrics = sessionMetrics; } /// @@ -139,7 +147,8 @@ public async Task SpawnAsync( chatClient, _toolAccessPolicy, _approvalService, - SubAgentMaxToolIterations); + SubAgentMaxToolIterations, + _sessionMetrics); var actorName = $"subagent-{definition.Name}-{runId}"; IActorRef subAgent; try diff --git a/src/Netclaw.Daemon.IntegrationTests/SkillServerNativeSidecarIntegrationTests.cs b/src/Netclaw.Daemon.IntegrationTests/SkillServerNativeSidecarIntegrationTests.cs index 1d5c6812a..e64ad7298 100644 --- a/src/Netclaw.Daemon.IntegrationTests/SkillServerNativeSidecarIntegrationTests.cs +++ b/src/Netclaw.Daemon.IntegrationTests/SkillServerNativeSidecarIntegrationTests.cs @@ -25,7 +25,7 @@ namespace Netclaw.Daemon.IntegrationTests; [Trait("Category", "Integration")] public sealed class SkillServerNativeSidecarIntegrationTests : IAsyncLifetime { - private const string Image = "ghcr.io/netclaw-dev/skillserver:0.4.0-beta.1"; + private const string Image = "ghcr.io/netclaw-dev/skillserver:0.4.0-beta.3"; private const string ApiKey = "sk-test-native-sidecar-sync"; private const int ContainerPort = 8080; private const int HostPort = 18080; diff --git a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs index b2a910026..76aa6b353 100644 --- a/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs +++ b/src/Netclaw.Daemon.Tests/Services/ServerFeedSkillSyncServiceTests.cs @@ -146,7 +146,7 @@ public async Task SyncOnce_missing_native_sidecar_preserves_rfc_skill_sync() """, "application/json"); handler.AddStringResponse(BaseUrl + "skills/feed-skill/1.0.0/SKILL.md", skillContent, "text/markdown"); - handler.AddErrorResponse(BaseUrl + "manifest.json", HttpStatusCode.NotFound); + handler.AddErrorResponse(BaseUrl + "subagents/v1/index.json", HttpStatusCode.NotFound); var service = CreateService(handler); await service.SyncOnceAsync(CancellationToken.None); @@ -412,72 +412,58 @@ private static void AddNativeSubAgentResponses( byte[] artifactContent, string expectedDigest) { + // Use absolute hrefs so the client resolves direct native index traversal correctly. handler.AddStringResponse( - BaseUrl + "manifest.json", - """ - { - "$schema": "https://schemas.netclaw.dev/skillserver/native-manifest/v1.json", - "generatedAt": "2026-06-30T00:00:00Z", - "links": { - "self": { "href": "manifest.json" }, - "rfcSkills": { "href": ".well-known/agent-skills/index.json" }, - "skills": { "href": "manifest/skills/index.json" }, - "subagents": { "href": "manifest/subagents/index.json" } - } - } - """, - "application/json"); - handler.AddStringResponse( - BaseUrl + "manifest/subagents/index.json", + BaseUrl + "subagents/v1/index.json", """ { "kind": "subagent-collection-index", - "links": { "self": { "href": "manifest/subagents/index.json" } }, + "links": { "self": { "href": "/subagents/v1/index.json" } }, "pages": [ - { "range": "a-z", "href": "manifest/subagents/pages/a-z.json" } + { "range": "a-z", "href": "/subagents/v1/pages/a-z.json" } ] } """, "application/json"); handler.AddStringResponse( - BaseUrl + "manifest/subagents/pages/a-z.json", + BaseUrl + "subagents/v1/pages/a-z.json", $$""" { "kind": "subagent-collection-page", "range": "a-z", - "links": { "self": { "href": "manifest/subagents/pages/a-z.json" } }, + "links": { "self": { "href": "/subagents/v1/pages/a-z.json" } }, "items": [ { "name": "{{name}}", "latestVersion": "{{version}}", "versionRange": { "min": "{{version}}", "max": "{{version}}", "count": 1 }, - "href": "manifest/subagents/{{name}}/index.json" + "href": "/subagents/v1/{{name}}/index.json" } ] } """, "application/json"); handler.AddStringResponse( - BaseUrl + $"manifest/subagents/{name}/index.json", + BaseUrl + $"subagents/v1/{name}/index.json", $$""" { "kind": "subagent-identity-index", "name": "{{name}}", "latestVersion": "{{version}}", - "links": { "self": { "href": "manifest/subagents/{{name}}/index.json" } }, + "links": { "self": { "href": "/subagents/v1/{{name}}/index.json" } }, "versions": [ { "version": "{{version}}", "publishedAt": "2026-06-30T00:00:00Z", "digest": "sha256:{{expectedDigest}}", - "href": "manifest/subagents/{{name}}/versions/{{version}}.json" + "href": "/subagents/v1/{{name}}/versions/{{version}}.json" } ] } """, "application/json"); handler.AddStringResponse( - BaseUrl + $"manifest/subagents/{name}/versions/{version}.json", + BaseUrl + $"subagents/v1/{name}/versions/{version}.json", $$""" { "kind": "subagent-version-detail", @@ -487,7 +473,7 @@ private static void AddNativeSubAgentResponses( "description": "Test sub-agent", "url": "{{BaseUrl}}subagents/{{name}}/{{version}}/agent.md", "digest": "sha256:{{expectedDigest}}", - "links": { "self": { "href": "manifest/subagents/{{name}}/versions/{{version}}.json" } } + "links": { "self": { "href": "/subagents/v1/{{name}}/versions/{{version}}.json" } } } """, "application/json"); diff --git a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs index 8b2026b8a..b850e93d5 100644 --- a/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs +++ b/src/Netclaw.Daemon/Services/ServerFeedSkillSyncService.cs @@ -1,4 +1,4 @@ -// ----------------------------------------------------------------------- +// ----------------------------------------------------------------------- // // Copyright (C) 2026 - 2026 Petabridge, LLC // @@ -373,16 +373,7 @@ private async Task SyncNativeSubAgentsAsync( using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); cts.CancelAfter(TimeSpan.FromSeconds(feed.TimeoutSeconds)); - var manifest = await client.GetManifestAsync(cts.Token); - if (manifest?.Links?.SubAgents is not { Href.Length: > 0 } subAgentsLink) - { - _logger.LogDebug( - "Server feed '{FeedName}' native sidecar is unavailable or does not advertise sub-agents", - feed.Name); - return; - } - - subAgentIndex = await client.GetNativeSubAgentIndexAsync(subAgentsLink, cts.Token); + subAgentIndex = await client.GetNativeSubAgentIndexAsync(cts.Token); } catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) {