Skip to content

feat(core,python): pin-aware resolution + recall pin boost (PRD-04 DEP-5, Part A) - #671

Open
p-diogo wants to merge 10 commits into
mainfrom
feat/dep-5-pin-aware-resolution
Open

p-diogo wants to merge 10 commits into
mainfrom
feat/dep-5-pin-aware-resolution

Conversation

@p-diogo

@p-diogo p-diogo commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Summary

PRD-04 F1 / DEP-5 (track kg, L2). Part A of 2 (Part B = MCP, after a core RC).

  • Core: RerankerConfig.pin_boost: Option<f64> (JSON pin_boost), Candidate.pinned (JSON pinned), DEFAULT_PIN_BOOST = 1.5; PyO3 rerank_with_config(..., pin_boost=None) + default_pin_boost(); WASM rerankWithConfig(..., pin_boost?) + defaultPinBoost(). Default is a no-op: every existing ranking is bit-for-bit identical (tests assert to_bits equality); no parity fixture changed.
  • Python: agent/pin_guard.py drops LLM UPDATE/DELETE whose target is pinned; unknown pin state fails closed (DELETE dropped, UPDATE → ADD). The contradiction resolver now receives entity refs + st:"p". It was dormant before: no entities were passed, so core never compared claims. It acts on existing_pinned only. Recall passes core's pin boost (feature-detected; no-op on core 2.6.x).
  • Docs: SKILL.md pin claim corrected; memory-taxonomy-v1.md §Pin semantics items 6–8; retrieval-v2.md §Pin boost; CLAUDE.md matrix + 3 Known Gaps; CHANGELOGs.

Decisions (see plan)

  • Pinned target → whole action dropped, matching core SkipNew{ExistingPinned} and the MCP/plugin contradiction path. Logs carry the category only; no decision-log row.
  • 1.5× = signed-off PinConfig::default().hard_boost. No boost without a query embedding: lexical-only ties would lift every pin to the top. Hermes auto-recall gets the boost with DEP-8.
  • Resolver outcomes other than existing_pinned stay log-only until DEP-12.
  • Candidate.pinned is a new public field: Rust struct-literal callers must add it. In-tree callers are fixed. Coordinator: confirm minor vs major for the core release.

Sequencing

Touches _auto_extract_inner in python/src/totalreclaw/agent/lifecycle.py: one import line plus one block after the contradiction try/except. DEP-3 edits the top of the same function. No fixed merge order with DEP-3: whichever merges second rebases.

Open items (New Feature Checklist)

  • Staging E2E python/tests/test_staging_pin_contract.py, run by the coordinator with a throwaway staging vault in the environment; it asserts the staging DataEdge.
  • Hermes real-user QA "pin, then contradict it three times in conversation" on the Python RC (PRD-04 G2).
  • Part B: MCP managed recall pin boost after the core RC.
  • Out of scope, flagged: python/src/totalreclaw/crystals/recrystallize.py rewrites and tombstones facts without preserving pin_status.

Test evidence

cargo test (core), cargo test --features python --lib python::tests, cargo check --tests (totalreclaw-memory), the CI Python suite, scripts/check-pyfunction-parity.py, scripts/check-docs.sh, scripts/check-phrase-safety.sh, and the WASM smoke (a,b,c / p,a,b / 1.5), all green locally.

🤖 Generated with Claude Code

p-diogo and others added 9 commits September 30, 2026 18:43
Co-Authored-By: Claude Code <noreply@anthropic.com>
… (PRD-04 DEP-5)

Co-Authored-By: Claude Code <noreply@anthropic.com>
…re pin boost (PRD-04 DEP-5)

Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Code <noreply@anthropic.com>
…04 DEP-5)

Co-Authored-By: Claude Code <noreply@anthropic.com>
…er (PRD-04 DEP-5)

Co-Authored-By: Claude Code <noreply@anthropic.com>
…RD-04 G2)

Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Code <noreply@anthropic.com>
@p-diogo

p-diogo commented Sep 30, 2026

Copy link
Copy Markdown
Owner Author

Whole-PR review (Sonnet): verdict FIX-FIRST

One fail-open hole in the pin guard; the rest of the PR is sound. CI is green on the checks that ran.

Blocker: get_fact_pin_status fails open on a GraphQL error body.
_fetch_fact_by_id (python/src/totalreclaw/operations.py:1151) reads data.get("data", {}).get("fact"), but relay.query_subgraph returns the parsed body for any HTTP 2xx. An indexer error arrives as {"errors":[…]} with no data key, which yields None, so get_fact_pin_status returns False (operations.py:1191) and the guard lets the UPDATE/DELETE through. A pinned fact can be tombstoned during a subgraph hiccup — the opposite of the guard's fail-closed contract.

Fix: query directly, raise when errors is present or data is not a dict containing fact, and return False only when data["fact"] is explicitly None. Add tests for {"errors":[…]} and {}; today's _Relay stub returns the fact directly, so the path is untested.

Confirmed sound

  • No-op default: pinned is #[serde(default, skip_serializing_if)] and pin_boost is an absent Option, so existing JSON parses and serialises identically. No parity fixture changes.
  • Bypass paths: the only automatic tombstone sites (lifecycle.py:399, :497) are both behind the guard. Contradiction auto-resolution never tombstones; it drops the new fact on existing_pinned.
  • Foreign fact ids: a live foreign fact fails decryption, so the guard fails closed. False comes back only for an inactive fact, where tombstoning is harmless.
  • DEP-3 collision (feat(core): topical dedup context — build_dedup_context + WASM/PyO3 bindings (PRD-04 DEP-3, Part A) #669): only textual conflicts in the registration blocks and the CHANGELOG. The WASM files differ. Whichever of DEP-5 Part B or DEP-3 Part B lands second rebases over the guard hunk at lifecycle.py:355-369.

Follow-ups (non-blocking, filed in the internal repo)

  1. Stale id after a pin. Pinning tombstones the old id and writes a new one. An LLM UPDATE aimed at the pre-pin id reads as inactive, so the update proceeds and leaves a non-pinned duplicate alongside the pinned fact. Nothing is tombstoned.
  2. Confirm that crystals/recrystallize.py:905/:930 (client.forget, user-invoked) cannot reach a pinned fact.
  3. rust/totalreclaw-memory/Cargo.lock is stale on main (it pins core 2.5.5 while the workspace is at 2.6.0). The worker reverted its refresh to keep the diff to plan. No --locked build of that crate runs in CI today, so commit a refresh separately.

…s (DEP-5)

relay.query_subgraph returns the parsed body for any HTTP 2xx, so a
GraphQL error body ({"errors": [...]}) carries no "data" key; the lenient
.get chain collapsed it to None, get_fact_pin_status returned False, and
the pin guard let an LLM UPDATE/DELETE tombstone a pinned fact during a
subgraph hiccup.

_fetch_fact_by_id gains a keyword-only strict mode, used only by
get_fact_pin_status: a non-empty "errors" key, or a body whose "data" is
not a dict carrying "fact", raises RuntimeError (pin state unknown ->
guard fails closed). False is returned only for an explicit
data.fact is None. Other callers keep the lenient shape unchanged.

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
@p-diogo

p-diogo commented Sep 30, 2026

Copy link
Copy Markdown
Owner Author

Blocker fixed in cc36fbb: get_fact_pin_status now fails closed. A 2xx body carrying errors, or one missing data/fact, raises instead of returning False; only an explicit data.fact = null still means 'not pinned'. _fetch_fact_by_id gained a keyword-only strict flag, so the pin/unpin and E2E callers are untouched. Red-then-green confirmed: the 4 new cases failed with DID NOT RAISE beforehand. Full Python suite 2577 passed (baseline 2572 + the 5 new tests), docs and phrase-safety guards clean, CI green (7 pass, 5 path-skipped).

The fix-up also surveyed the same data.get("data", {}) shape elsewhere. No other path can reach the guard, so nothing else is destructive, but five non-pin sites turn a subgraph error into an empty result — including silent export truncation. Tracked as totalreclaw-internal#559, with a shared relay.py parser as the suggested fix. Ready to merge.

This branch has not been deployed

No deployments
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