Skip to content

feat(daemon): capture fired-helpful via a new used_helpfully signal#20

Merged
nickroci merged 3 commits into
mainfrom
feat/fired-helpful-capture
May 29, 2026
Merged

feat(daemon): capture fired-helpful via a new used_helpfully signal#20
nickroci merged 3 commits into
mainfrom
feat/fired-helpful-capture

Conversation

@nickroci

Copy link
Copy Markdown
Owner

What

Wires up capture of the fired-helpful counter on knowledge entries. It was a dead frontmatter field — present on every entry (always 0) and named in the prompt schemas, but nothing ever incremented it. Now it gets bumped when the assistant actually relied on a surfaced memory in a turn.

Scope is capture only. fired-helpful is not yet wired into retrieval ranking or decay — that's a deliberate, separate decision. This PR just counts the signal correctly.

Design

  • Librarian judges valence (not a regex): a new salience_signal: "used_helpfully" that fires only on positive reliance ("yes, as X says…"). Mere mention isn't use; disagreement stays contradicts and routes to the existing update path — it never bumps this counter.
  • Scholar writes, Librarian stays read-only — single-writer invariant on the knowledge dir preserved. The bump is applied deterministically in apply_fired_helpful_counters (mirrors apply_reinforcement_counters), not an LLM tool.
  • No double-counting. The rolling buffer is never drained, so a turn is re-scanned on every later Stop. The Librarian's scan-local turn_id is not stable across scans, so a new monotonic turn_seq is stamped at seal time in buffer.py. Dedup is a persisted per-(session, entry) high-water mark on cited_turn_seq (~/.agent-mem/daemon.fired-helpful.json, offset-state idiom): only seqs above the mark count; coalesced Stops count the whole gap; the mark advances only on a successful write.

Known minor edge (non-blocking)

If the entry-file bump succeeds but the state-file persist fails (rare; same volume), the next review can re-count that turn — an over-count. Benign while this is telemetry-only; worth hardening (persist mark before bump) if/when it feeds ranking.

Reconciliation

Reverse-merged current main (incl. PR #19 reflective-abstraction / abstract_entries) into this branch. The two features don't collide — one adds an action, this adds a salience signal. Both coexist; merge has no remaining conflicts.

Verification

  • Pyright (source): 0 errors (the lone __main__.py warning is pre-existing).
  • Pytest: 623 passed (599 here + 24 from reflective-abstraction), coverage 90.30%.

🤖 Generated with Claude Code

nickroci and others added 3 commits May 29, 2026 19:34
Wire up the previously-dead `fired-helpful` counter so it increments when
the agent actually RELIES ON a surfaced/cited memory entry in a turn.
Mirrors the existing reinforcement-counter pattern: the Librarian (read-
only) judges and PROPOSES; the Scholar (sole writer) applies the bump
deterministically. `fired-helpful` stays conceptually distinct from
`reinforced` (use vs. content re-assertion) and from `contradicts`
(entry is wrong — must never bump fired-helpful).

New salience signal
-------------------
Add a 4th `salience_signal` value `"used_helpfully"` on `_BaseAction`
(_schemas.py). It cites the relied-upon entry in `existing_entry` (as
`reinforces` does) and the turn it was used in via a new structural
`cited_turn_seq: Optional[int]` field. The Librarian prompt gains a
USED-HELPFULLY block (mutually exclusive with contradicts) and the Scholar
prompt gains a matching judgement note ("counter already bumped server-
side"). `cited_turn_seq` is excluded from the per-action payload-field
table like the other base fields.

Scholar-side apply
------------------
`scholar_prompt.apply_fired_helpful_counters` walks packets for
`used_helpfully` proposals, validates the path is inside the knowledge
dir, and bumps `fired-helpful` + stamps `last_fired_helpful: <today>` via
`_bump_fired_helpful_counter` (atomic tmp+rename, same as the reinforced
bump). `scholar._bump_fired_helpful_counters` calls it pre-model alongside
the reinforcement bump.

Double-counting prevention (the subtle part)
--------------------------------------------
The rolling buffer is never drained, so a physical turn is re-scanned on
every later Stop until it ages out — a naive bump would over-count one use
by up to deque depth.

KEY DATA-MODEL FINDING: the Librarian's `flatten_buffer` `turn_id` is a
per-EVENT counter recomputed FRESH each scan; it shifts down as old turns
age out, so it is NOT stable across scans. The task's suggested keys
(high-water turn_id, or a (session,turn_id,entry) dedup set) are unsound
against it. Root-cause fix instead: give each sealed turn a STABLE,
monotonic per-session `turn_seq` in the buffer (assigned at seal, survives
eviction), thread it into the snapshot, render it as a `(turn_seq=S)`
token the Librarian cites in `cited_turn_seq`.

Dedup = a persisted per-(session_id, entry_path) HIGH-WATER mark on
cited_turn_seq at ~/.agent-mem/daemon.fired-helpful.json (offset-state
idiom: atomic tmp+rename JSON, _load/_save). Only seqs strictly greater
than the mark count; within a batch every DISTINCT new seq counts
(coalesced Stops count the whole gap, not 1), then the mark advances to
the max — but only if the on-disk bump succeeded. Per-(session,entry)
keying (not per-session) so a new use of a different entry in an older-
still-in-window turn isn't suppressed. A used_helpfully lacking a valid
cited_turn_seq is dropped (logged) rather than counted, since it can't be
deduped — drop over over-count.

Restart edge (documented in code): the in-memory buffer + turn_seq
allocator reset on restart and the tailer resumes past old events, so the
over-count cannot occur across a restart; a session continuing across a
restart could under-count at most one bump per entry — benign and rare.

Note: no ModelRetry boundary check added for missing cited_turn_seq — the
Librarian validator is well-formedness-only ("recall over precision"), and
the apply step is the gate.

Tests (tests stay at pyright basic; source passes strict, 0 errors):
- buffer: turn_seq monotonic / stable-across-eviction / per-session /
  in-snapshot.
- librarian_prompt: 5-tuple + `(turn_seq=S)` line format; seq defaults to
  0 when absent.
- scholar_prompt: bump-once+stamp; contradicts does NOT bump; DOUBLE-COUNT
  regression (same (turn,entry) across two passes -> exactly one bump);
  new-turn-after-high-water counts; coalesced-stops count whole gap;
  repeated-seq-in-batch dedup; per-(session,entry) high-water; missing
  cited_turn_seq skipped; path-traversal rejected; high-water persists
  across reload.

Full daemon suite: 599 passed, coverage 90.12% (gate 90%); ruff clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Upgrade the prefrontal-inhibition roadmap row from TODO to "Partial":
the positive-use capture (used_helpfully signal -> fired-helpful counter,
per-turn deduped) shipped in this PR. Add an explicit TODO that actually
consuming the signal (decay resistance / retrieval ranking) is not yet
built, and the "surfaced but ignored" negative half is still unbuilt.

Also document the used_helpfully signal in the salience section (as
use-tracking, distinct from the three write-gating signals), add it to
the architecture diagram alongside the fired-helpful counter bump, and
refresh the test count (623 daemon, 865 total).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nickroci nickroci merged commit fe1adb6 into main May 29, 2026
4 checks passed
@nickroci nickroci deleted the feat/fired-helpful-capture branch May 29, 2026 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant