diff --git a/.claude-plugin/skill-assets.sha256 b/.claude-plugin/skill-assets.sha256 index e472307d..6c837646 100644 --- a/.claude-plugin/skill-assets.sha256 +++ b/.claude-plugin/skill-assets.sha256 @@ -2,5 +2,5 @@ 94bfa06317a8fe6a6a7e204bb70c5abdc9e4bbc34d79dd6f8447a30140bc8b85 .claude-plugin/plugin.json 055655db84af07561d002f0c69744313d8413c39f3e873f941f0fa0b1e76dc66 skills/engraphis-memory/references/CONVENTIONS.md 62019760766ff472a76a0f81437898f39e3c1fe2631732b7b7733e50c1ad837f skills/engraphis-memory/references/SCOPING.md -1f62ba2b6abf3dab266d5b4d9c85f2d7fd7fe4ece4e85e2413cfc3fe460ef2c1 skills/engraphis-memory/references/TOOLS.md +fcb5b4d939bda7d18e4b75c0e9106df4240aa7f8eabe8218e4e41f0038fa3df8 skills/engraphis-memory/references/TOOLS.md 0f98098df695b9a00dc78402911124ebf09a4a058f6c8bec2c6234ec61fac13a skills/engraphis-memory/SKILL.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 1920e79f..5a9d5f45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,68 @@ All notable changes to Engraphis are documented here. Format loosely follows ### Added +- Smart MCP `engraphis_recall_context` default `k` raised 8 -> 50 so the token-budget + packer binds on realistic stores by default. Measured at budget=1024 against a + 49-fact store: 100% labelled-relevance retention and ~50% of the store withheld + (savings_ratio 0.0 -> 0.4975) with no caller-side arguments. The packer is the + existing 1.6 contract; the change just makes it the default fast path. +- Smart MCP `engraphis_remember` now accepts and forwards `subject_key` and + `claim_kind` to the classic tool. Without this, every keyed write silently stored + empty keys because the served gateway surface dropped the parameters; the + documented safe-supersession mechanism is now reachable through MCP. +- A new integration at `integrations/commandcode/session_start_hook.py` (with + `scripts/install_cc_hook.py` for idempotent user-scope install/uninstall) wires + durable-memory recall into Command Code's SessionStart lifecycle: each new + session's first turn receives bounded relevant context as `additionalContext`. + Fail-open and silent on any error. Override workspace via + `ENGRAPHIS_HOOK_WORKSPACE`; override the MCP URL via `ENGRAPHIS_MCP_URL`. +- Cross-encoder reranker (`cross-encoder/ms-marco-MiniLM-L-6-v2`) is now + reachable as an opt-in config knob (`rerank_model=` on `MemoryEngine.create` + / `ENGRAPHIS_RERANK_MODEL`). Evaluated offline on the bundled retrieval gates + (sample.jsonl, codemem.jsonl, k=5): hit@5 stays at 1.0 with zero per-question + regressions, MRR@5 lifts 0.889 -> 0.944 (sample) and 0.962 -> 0.981 (codemem), + with ~15 ms per query added. Not the default; flip with a one-line config. + +### Changed + +- The reworded-correction detector in `core/resolve.py` now supersedes reworded + corrections without a stable `subject_key` when the aligned token diff shows + a same-attribute value change (e.g. "the timeout is 30 seconds" -> "we raised + the timeout to 90 seconds"). The strong-evidence branch and the rewrite_gate + branch both require a change marker (e.g. "now", "raised") to be accompanied + by a value_swap on the same shared subject, so a bare "now" can never retire + a fact it merely shares surface nouns with. Vetoes preserve coexisting + distinct facts: clashing environment qualifiers (staging vs production, + folded through `prod`/`production` and `dev`/`development` aliases so a + legitimate correction across short forms does not get vetoed), + named mixed-case identifier swaps (ProviderA -> ProviderB), and clean + noun-for-noun replacements (REST -> GraphQL docs). Measured on the + reproducible corpus shipped at + `eval/datasets/resolver_reworded_corrections.jsonl` (44 pairs, 38 + positives + 6 negatives); reproduce locally with + `python -m eval.resolver_reworded_corrections` or + `python -m eval.resolver_reworded_corrections --strict` in CI. +- The `temporal_splice` flag passed from `core/engine.py` to `resolve()` is + now narrowed to the bi-temporal backfill case (a deliberate `valid_at` + AND a `subject_key`), instead of any `valid_at`-pinned write. Scheduled + future writes stay on the present-time veto contract. + +### Fixed + +- The Smart MCP gateway `engraphis_remember` binding was silently dropping + `subject_key` and `claim_kind`; this is the underlying cause of the + benchmark correction-miss pattern that the reworded-correction detector + then had to compensate for. + +### Operational + +- The new `engraphis_recall_context` tool emits one `INFO` log per call with + workspace, k, budget, packed/omitted counts, and the call's measured ms. + Operators get visibility without changing the on-the-wire contract. + The standalone \engraphis-mcp-http\ launcher only configures the root logger when + \ENGRAPHIS_MCP_LOG\ is set to a truthy value (\ / \ rue\ / \yes\ / \info\ / + \on\); the default stays silent so the CLI keeps its quiet profile. + - The graph's "Show all nodes" toggle is replaced by a dedicated **Every node** layout built on a new ultra-performance engine (`engraphis-graph-every.js` + `engraphis-graph-every-worker.js`, WebGL2-only): all geometry is uploaded once and camera @@ -125,14 +187,14 @@ All notable changes to Engraphis are documented here. Format loosely follows ### Fixed -- The Every node dashboard view no longer crashes on open: a declaration-order bug in the - renderer threw during construction before anything painted. The scene canvas also keeps its - accessible role/label now instead of being hidden from assistive technology. -- Import previews now page the source manifest exactly like execution, so vaults whose manifest - outgrew one list page (10k identities) no longer show manifest-only files as silently absent - from the preview plan; beyond-boundary rows are reported as `missing` instead of dropped. - Manifest pages now use one read snapshot and de-duplicate identities that move across a - cursor while a concurrent import updates their path. +- The Every node dashboard view no longer crashes on open: a declaration-order bug in the + renderer threw during construction before anything painted. The scene canvas also keeps its + accessible role/label now instead of being hidden from assistive technology. +- Import previews now page the source manifest exactly like execution, so vaults whose manifest + outgrew one list page (10k identities) no longer show manifest-only files as silently absent + from the preview plan; beyond-boundary rows are reported as `missing` instead of dropped. + Manifest pages now use one read snapshot and de-duplicate identities that move across a + cursor while a concurrent import updates their path. - Importing more than 1,000 files through the dashboard no longer fails with "Internal Server Error": wizard upload routes parse multipart forms under the advertised 1,500-file ceiling instead of Starlette's hidden 1,000-part parser default, oversized batches return a clear 413, @@ -178,7 +240,17 @@ All notable changes to Engraphis are documented here. Format loosely follows now guards an unknown baseline instead of reporting spurious misses, denial-guard supersession binds digests computed from the parsed record rather than raw input, import-job finalization is generation-guarded so a stale worker cannot finalize over a - newer attempt, and the finalized-state check completes in constant time. + newer attempt, and the finalized-state check completes in constant time. +- Smart MCP `engraphis_session` now accepts `action="start_session"` and `action="end_session"` + (the full tool-name forms the Command Code harness sends when translating the AGENTS.md + `engraphis_start_session`/`engraphis_end_session` shorthand), normalizing them to `start`/`end` + before the pattern validation instead of rejecting them with a 400. + +### Documentation + +- `docs/LLM_PROVIDERS.md` now warns Windows users that `cmd` may resolve to `cmd.exe` + (the built-in Windows command interpreter) instead of the Command Code CLI, and explains + how to diagnose and work around the PATH collision. ### Security diff --git a/engraphis/core/engine.py b/engraphis/core/engine.py index b3cee190..444e8845 100644 --- a/engraphis/core/engine.py +++ b/engraphis/core/engine.py @@ -2175,9 +2175,15 @@ def append_visible_neighbors( for sim, rec in extra_neighbors: if rec.id not in known_ids: neighbors.append((sim, rec)) + # Bi-temporal backfill only: anchored writes (valid_at pinned AND a + # subject_key is present) assert explicit chain membership and may + # supersede a live neighbour even when prose alone would suggest two + # coexisting facts. Other valid_at-pinned writes (e.g. scheduled + # future writes) stay on the present-time veto contract. decision = resolve( text, neighbors, subject_key=subject_key, claim_kind=claim_kind, candidate_content=content, + temporal_splice=valid_at is not None and bool(subject_key), ) # Repair trigger: when the resolver cannot safely supersede (INVALIDATE/NOOP), # surface a genuine high-severity contradiction as a persisted relation instead diff --git a/engraphis/core/resolve.py b/engraphis/core/resolve.py index 26fa759c..a1672e11 100644 --- a/engraphis/core/resolve.py +++ b/engraphis/core/resolve.py @@ -8,6 +8,17 @@ on the text itself supplies a precise, embedder-independent signal. An LLM-backed resolver can be plugged in later behind the same ``resolve()`` signature without touching callers. +For unkeyed reworded corrections — the case a lexical hashing embedder cannot score on +cosine alone — a third deterministic signal narrows the gap: an aligned token diff +(``difflib.SequenceMatcher``) between candidate and neighbor. A replace block whose two +sides carry disjoint changed numbers/dates of the same kind, anchored by a shared +neighbouring token (a *value swap*), or an explicit change marker in the candidate +("switched", "rescheduled", "increased", ...) upgrades same-subject overlap from +``RELATE`` to ``INVALIDATE``. Environment qualifiers (staging vs production), named +mixed-case identifiers in a swap (iOS -> Android), and clean noun-for-noun replacements +with no changed value (REST -> GraphQL docs, API runtime -> worker runtime) are vetoes: +those pairs are genuinely distinct facts and must both stay live. + It deliberately collapses the original design's UPDATE and INVALIDATE into one ``INVALIDATE`` ("supersede") operation — close the old fact's validity, add the new one — because both must preserve history under the non-negotiable "never overwrite" rule (AGENTS.md §3.2), @@ -17,12 +28,14 @@ from __future__ import annotations from dataclasses import dataclass +from difflib import SequenceMatcher from enum import Enum +import re import unicodedata from typing import Optional from engraphis.core.interfaces import MemoryRecord -from engraphis.core.textutil import jaccard, tokenize +from engraphis.core.textutil import _STOPWORDS, jaccard, tokenize # Embedding-similarity floor: skip the (cheap but not free) token-overlap check for # neighbors the vector index itself considers unrelated. The real decision is below. @@ -41,6 +54,17 @@ AMBIGUITY_EPSILON = 1e-9 AMBIGUITY_MARGIN = 0.05 +# Reworded-correction legs (unkeyed text only). Agreement bar: the pair shares enough +# surviving subject matter. The default bar is folded Jaccard >= REWRITE_JACCARD or +# folded containment of the smaller side >= REWRITE_CONTAINMENT (containment tolerates +# inflection drift like page/paging); an explicit change marker lowers the containment +# bar to REWRITE_MARKER_CONTAINMENT because the writer asserted the replacement, and a +# confident semantic cosine plus a marker opens the gate for embedders that survive +# heavy rewording. +REWRITE_JACCARD = 0.40 +REWRITE_CONTAINMENT = 0.40 +REWRITE_MARKER_CONTAINMENT = 0.20 + # Relation persisted on ``mem_links`` when the deterministic detector finds a genuine # high-severity contradiction that the resolver cannot safely supersede (no shared # claim key and not enough joint lexical/semantic evidence). ``conflicts_with`` is a @@ -50,6 +74,66 @@ # the generic SEMANTIC overlay, which is the correct conservative default. CONFLICT_RELATION = "conflicts_with" +_CHANGE_MARKERS = frozenset({ + "switched", "moved", "migrated", "replaced", "rescheduled", "relocated", + "upgraded", "downgraded", "transferred", "renamed", "ported", "increased", + "decreased", "raised", "lowered", "bumped", "extended", "reduced", "expanded", + "changed", "grew", "resized", "retired", "deprecated", "instead", "now", +}) +_LIGHT_TOKENS = frozenset({ + "use", "used", "using", "run", "ran", "set", "get", "go", "went", +}) | _CHANGE_MARKERS +_ENV_QUALIFIERS = frozenset({ + "staging", "production", "prod", "development", "dev", "test", "testing", + "qa", "uat", "preview", "sandbox", "demo", "local", +}) +# Aliases for the same logical environment so a write of "prod" and a record +# of "production" do not look like two distinct environments to the conflict +# veto. Tokens map to a single canonical form; a non-empty intersection +# between two canonical sets therefore means both sides refer to the same +# environment, and the "env_conflict" veto only fires when the canonical +# sets are non-empty on both sides AND disjoint. +_ENV_ALIASES: dict[str, str] = { + "prod": "production", + "production": "production", + "dev": "development", + "development": "development", + "test": "test", + "testing": "test", + "qa": "qa", + "uat": "qa", + "staging": "staging", + "preview": "preview", + "sandbox": "sandbox", + "demo": "demo", + "local": "local", +} + + +def _canonical_env(tokens: set[str]) -> set[str]: + """Fold env aliases to one canonical form per logical environment. + + A bare ``prod`` and a bare ``production`` are the same logical + environment; folding them prevents a legitimate correction ("Prod API + timeout is 30s" -> "Production API timeout increased to 90s") from being + vetoed by ``env_conflict``. + """ + return {_ENV_ALIASES.get(token, token) for token in tokens} + + +_MONTHS = frozenset({ + "january", "february", "march", "april", "may", "june", "july", + "august", "september", "october", "november", "december", +}) +_WEEKDAYS = frozenset({ + "monday", "tuesday", "wednesday", "thursday", "friday", "saturday", "sunday", +}) +_NUMBER_WORDS = frozenset({ + "one", "two", "three", "four", "five", "six", "seven", "eight", "nine", + "ten", "eleven", "twelve", "hundred", "thousand", "million", "billion", +}) +_ORDINAL_RE = re.compile(r"\d{1,10}(?:st|nd|rd|th)\Z") + def _normalise_claim_text(value: str) -> str: """Compare keyed claims independent of whitespace and terminal punctuation. @@ -74,6 +158,38 @@ def _normalise_claim_text(value: str) -> str: ).split() ).casefold() + +def _fold(token: str) -> str: + """Naive singular/plural fold so page/pages-style drift still aligns.""" + return token[:-1] if len(token) > 3 and token.endswith("s") else token + + +def _surface_tokens(text: str) -> list[tuple[str, bool]]: + """Ordered ``(folded_token, is_named_identifier)`` pairs for diff alignment. + + Mirrors ``tokenize``'s filtering (same stopword set; short tokens dropped, + except digit-bearing ones, which carry the values corrections change) and adds + a named flag: a token with an uppercase letter past its first character that is + not an all-caps acronym (``ProviderA``, ``iOS``, ``v2Beta``, but not ``API``). + Such tokens are usually identifiers whose replacement signals distinct facts + rather than a corrected value. + """ + out: list[tuple[str, bool]] = [] + for surface in re.findall(r"[A-Za-z0-9]+", str(text or "")): + lowered = surface.lower() + named = ( + any(character.isupper() for character in surface[1:]) + and not surface.isupper() + ) + folded = _fold(lowered) + if folded in _STOPWORDS: + continue + if len(folded) <= 1 and not any(c.isdigit() for c in folded): + continue + out.append((folded, named)) + return out + + class ResolutionOp(str, Enum): ADD = "add" # genuinely new -> insert NOOP = "noop" # already known -> reinforce the existing memory, don't insert @@ -88,9 +204,22 @@ class Resolution: reason: str = "" +@dataclass(frozen=True) +class CorrectionEvidence: + """Deterministic diff verdict on whether a candidate rewrites a neighbour.""" + + marker: bool + value_swap: bool + proper_swap: bool + heavy_swap: bool + env_conflict: bool + shared_subject: int = 0 + + def resolve(candidate_text: str, neighbors: list[tuple[float, MemoryRecord]], *, subject_key: str = "", claim_kind: str = "", - candidate_content: Optional[str] = None) -> Resolution: + candidate_content: Optional[str] = None, + temporal_splice: bool = False) -> Resolution: """Decide ADD / NOOP / INVALIDATE for new content against its nearest neighbors. ``neighbors`` are ``(embedding_similarity, MemoryRecord)`` pairs that the caller has @@ -148,6 +277,7 @@ def resolve(candidate_text: str, neighbors: list[tuple[float, MemoryRecord]], *, return Resolution(ResolutionOp.ADD, reason="no related memory in scope") overlap, rec, sim = best + rec_text = f"{rec.title} {rec.content}" same_subject = bool(candidate_subject) and candidate_subject == ( str(rec.subject_key or "").strip() ) @@ -192,11 +322,27 @@ def resolve(candidate_text: str, neighbors: list[tuple[float, MemoryRecord]], *, ) return Resolution(ResolutionOp.NOOP, target_id=rec.id, reason=f"near-duplicate of {rec.id} (token overlap={overlap:.2f})") - # Without an explicit claim key, invalidation needs strong agreement from - # lexical and semantic signals. A high cosine alone can be a topical - # neighbor rather than a contradiction, so it does not change either fact. - if (not candidate_subject and overlap >= STRONG_SUBJECT_TOKEN_JACCARD - and sim >= STRONG_JOINT_EMBED_SIM): + # Without an explicit claim key, invalidation needs agreement from the lexical + # and semantic signals plus — for reworded corrections — diff evidence that a + # value actually changed. A clean noun-for-noun replacement with no changed + # value ("runtime for the API service" vs "for the worker service", REST docs + # vs GraphQL docs) vetoes even strong joint evidence: the pair is two distinct + # facts about one topic, not a correction. + evidence: Optional[CorrectionEvidence] = None + strong = (not candidate_subject and overlap >= STRONG_SUBJECT_TOKEN_JACCARD + and sim >= STRONG_JOINT_EMBED_SIM) + containment = _containment(cand_tokens, rec_text) + marker = _has_marker(candidate_text) + rewrite_gate = ( + not candidate_subject and sim >= RELATED_SIM_FLOOR + and (overlap >= REWRITE_JACCARD + or containment >= REWRITE_CONTAINMENT + or (containment >= REWRITE_MARKER_CONTAINMENT and marker) + or (sim >= STRONG_JOINT_EMBED_SIM and marker)) + ) + if strong or rewrite_gate: + evidence = _correction_evidence(candidate_text, rec_text) + if strong: ambiguous = [ item for item in scored[1:] if item[0] >= STRONG_SUBJECT_TOKEN_JACCARD @@ -210,11 +356,194 @@ def resolve(candidate_text: str, neighbors: list[tuple[float, MemoryRecord]], *, reason=f"ambiguous strong match among {ids}; no memory superseded " f"(best overlap={overlap:.2f}, similarity={sim:.2f})", ) - return Resolution(ResolutionOp.INVALIDATE, target_id=rec.id, - reason=f"supersedes {rec.id} (strong joint evidence: " - f"token overlap={overlap:.2f}, similarity={sim:.2f})") + # Swap vetoes protect *live* neighbours on ordinary present-time writes: + # a heavy or named-identifier swap against a live fact reads as two + # coexisting facts. An explicitly time-anchored write asserts chain + # membership (bi-temporal splice), and a closed predecessor is already + # historical — strong joint evidence supersedes both regardless of prose. + # Clashing environment qualifiers (staging vs production) always veto, + # matching the contract honoured by the rewrite_gate branch below: a + # strong overlap between a staging fact and a production fact is two + # coexisting truths, not a single fact being corrected. The marker + # override on proper_swap requires a value_swap alongside the marker + # so a bare "now" can never retire a fact it merely shares surface + # nouns with. + assert evidence is not None # strong => evidence was computed above + swap_veto = (evidence.heavy_swap + or (evidence.proper_swap and not (marker and evidence.value_swap)) + or evidence.env_conflict) + if not swap_veto or temporal_splice or rec.valid_to is not None: + return Resolution(ResolutionOp.INVALIDATE, target_id=rec.id, + reason=f"supersedes {rec.id} (strong joint evidence: " + f"token overlap={overlap:.2f}, similarity={sim:.2f})") + if rewrite_gate and evidence is not None and not evidence.env_conflict: + # A bare change marker ("now", "actually", ...) on a candidate that + # shares no subject tokens with the neighbour is not correction + # evidence — common words leak into every sentence. Require the + # same shared-subject floor that the value-swap branch uses, so the + # marker can only lift a candidate that already overlaps on the + # same subject. + marker_corrected = ( + evidence.marker + and evidence.value_swap + and not evidence.proper_swap + and not evidence.heavy_swap + and evidence.shared_subject >= 2 + ) + value_corrected = ( + evidence.value_swap + and evidence.shared_subject >= 2 + and not evidence.proper_swap + and not evidence.heavy_swap + ) + if marker_corrected or value_corrected: + if marker_corrected: + kind = "change marker" + else: + kind = "value change" + return Resolution( + ResolutionOp.INVALIDATE, target_id=rec.id, + reason=f"supersedes {rec.id} (reworded correction by {kind}: " + f"token overlap={overlap:.2f}, similarity={sim:.2f})", + ) if overlap >= SUBJECT_TOKEN_JACCARD: return Resolution(ResolutionOp.RELATE, target_id=rec.id, reason=f"related to {rec.id} (same topic, " f"token overlap={overlap:.2f}, similarity={sim:.2f})") return Resolution(ResolutionOp.ADD, reason=f"related but distinct (best overlap={overlap:.2f})") + + +def _containment(cand_tokens: set[str], rec_text: str) -> float: + """Share of the smaller folded token set that survives across the pair.""" + cand_folded = {_fold(token) for token in cand_tokens} + rec_folded = {_fold(token) for token in tokenize(rec_text)} + if not cand_folded or not rec_folded: + return 0.0 + return len(cand_folded & rec_folded) / min(len(cand_folded), len(rec_folded)) + + +def _has_marker(text: str) -> bool: + return bool({word for word in re.findall(r"[a-z]+", str(text or "").lower())} + & _CHANGE_MARKERS) + + +def _is_value(token: str) -> bool: + """Numbers, dates (incl. month/weekday names and spelled-out small counts).""" + return ( + any(character.isdigit() for character in token) + or bool(_ORDINAL_RE.fullmatch(token)) + or token in _MONTHS or token in _WEEKDAYS or token in _NUMBER_WORDS + ) + + +def _value_kind(token: str) -> str: + """Coarse value class so "budget 50k" never swaps against "deadline March 15".""" + if token in _MONTHS or token in _WEEKDAYS: + return "cal" + if _ORDINAL_RE.fullmatch(token): + return "ord" + if any(character.isdigit() for character in token): + return "num" + return "numword" + + +_SWAP_SPAN = tuple[tuple[int, int], tuple[int, int]] + + +def _swap_spans(cand_words: list[str], rec_words: list[str]) -> list[_SWAP_SPAN]: + """Index spans of aligned replace blocks. + + SequenceMatcher with ``autojunk=False`` never emits adjacent delete+insert + pairs for genuine value swaps (it always uses ``replace``), so we rely on + the ``replace`` opcode alone and skip the merge. + """ + matcher = SequenceMatcher(None, cand_words, rec_words, autojunk=False) + return [((i1, i2), (j1, j2)) for op, i1, i2, j1, j2 + in matcher.get_opcodes() if op == "replace"] + + +def _anchor_ok(cand: list[tuple[str, bool]], rec: list[tuple[str, bool]], + old_span: tuple[int, int], new_span: tuple[int, int]) -> bool: + """True when a value swap shares a neighbouring token across both sides. + + The neighbours of the changed *values themselves* (within +/-1 of each value + position) approximate the attribute being re-valued: "2 replicas -> 6 + replicas" shares ``replicas``; "budget 50k -> deadline March 15" shares only + sentence furniture ("per", "the"), because the attribute itself changed — a + distinct fact, not a correction. The tight radius keeps shared subject nouns + ("search index", "staging database") outside the window. + """ + def neighbourhood(seq: list[tuple[str, bool]], span: tuple[int, int]) -> set[str]: + positions: list[int] = [] + for index in range(*span): + if _is_value(seq[index][0]): + positions.extend([index - 1, index, index + 1]) + return {seq[i][0] for i in positions if 0 <= i < len(seq)} + + return bool(neighbourhood(cand, old_span) & neighbourhood(rec, new_span)) + + +def _correction_evidence(candidate_text: str, record_text: str) -> CorrectionEvidence: + """Deterministic diff evidence for (or against) a reworded correction. + + Inspects the aligned replace blocks: a *value swap* changes disjoint + numbers/dates of the same kind with a shared attribute neighbour; a *heavy + swap* replaces plain nouns with no value involved — the signature of two + distinct facts. Named mixed-case identifiers inside a value block set + ``proper_swap`` (ProviderA -> ProviderB beside 4 -> 8 workers), and clashing + environment qualifiers (staging vs production) set ``env_conflict``; both veto + the value-swap leg. + """ + cand = _surface_tokens(candidate_text) + rec = _surface_tokens(record_text) + cand_words = [token for token, _ in cand] + rec_words = [token for token, _ in rec] + env_a = {token for token, _ in cand if token in _ENV_QUALIFIERS} + env_b = {token for token, _ in rec if token in _ENV_QUALIFIERS} + env_a_canon = _canonical_env(env_a) + env_b_canon = _canonical_env(env_b) + env_conflict = bool(env_a_canon and env_b_canon and env_a_canon.isdisjoint(env_b_canon)) + + value_swap = False + proper_swap = False + heavy_swap = False + for old_span, new_span in _swap_spans(cand_words, rec_words): + old_pairs = cand[old_span[0]:old_span[1]] + new_pairs = rec[new_span[0]:new_span[1]] + old_values = [token for token, _ in old_pairs if _is_value(token)] + new_values = [token for token, _ in new_pairs if _is_value(token)] + if old_values and new_values: + if (set(old_values).isdisjoint(new_values) + and _value_kind(old_values[0]) == _value_kind(new_values[0]) + and _anchor_ok(cand, rec, old_span, new_span)): + value_swap = True + if any(named for _, named in [*old_pairs, *new_pairs]): + proper_swap = True + elif old_pairs and new_pairs: + # Env-alias tokens (prod/production, dev/development, ...) fold + # to the same canonical form, so swapping one for the other is + # not a noun-for-noun replacement — exclude them from the + # heavy_swap count so legitimate corrections like "prod API + # timeout is 30s" -> "production API timeout increased to 90s" + # are not vetoed as coexisting facts. + old_heavy = [token for token, _ in old_pairs + if token not in _LIGHT_TOKENS + and not _is_value(token) + and token not in _ENV_QUALIFIERS] + new_heavy = [token for token, _ in new_pairs + if token not in _LIGHT_TOKENS + and not _is_value(token) + and token not in _ENV_QUALIFIERS] + if old_heavy and new_heavy: + heavy_swap = True + + def _subject_tokens(pairs: list[tuple[str, bool]]) -> set[str]: + return {token for token, _ in pairs + if token not in _LIGHT_TOKENS and not _is_value(token)} + + shared_subject = len(_subject_tokens(cand) & _subject_tokens(rec)) + return CorrectionEvidence( + marker=_has_marker(candidate_text), value_swap=value_swap, + proper_swap=proper_swap, heavy_swap=heavy_swap, + env_conflict=env_conflict, shared_subject=shared_subject, + ) diff --git a/engraphis/mcp_http_cli.py b/engraphis/mcp_http_cli.py index a9857ebf..ad90967c 100644 --- a/engraphis/mcp_http_cli.py +++ b/engraphis/mcp_http_cli.py @@ -1,148 +1,171 @@ -"""Console entry for a local loopback MCP-over-HTTP server. - -This is intentionally a generic MCP transport, not an integration with any particular -agent host. Remote MCP access belongs behind the authenticated dashboard ``/mcp`` -mount; a standalone FastMCP transport has no Engraphis authentication middleware. -""" -from __future__ import annotations - -import argparse -import importlib.util -import ipaddress -import os -import sys - -_TRANSPORTS = ("streamable-http", "sse") - - -def _dependency_error() -> str: - if sys.version_info < (3, 10): - return ( - "The Engraphis MCP server requires Python 3.10 or newer.\n" - "Create a Python 3.10+ environment, then run: pip install \"engraphis[mcp]\"" - ) - if importlib.util.find_spec("mcp") is None: - return ( - "The 'mcp' package is required to run the Engraphis MCP server.\n" - "Install it with: pip install \"engraphis[mcp]\"" - ) - return "" - - -def _loopback_host(value: str) -> str: - host = value.strip() - # Accept 'localhost' as a synonym for 127.0.0.1 — standard network tool behavior. - if host.lower() == "localhost": - return "127.0.0.1" - try: - if ipaddress.ip_address(host).is_loopback: - return host - except ValueError: - pass - raise argparse.ArgumentTypeError( - "standalone MCP-over-HTTP accepts loopback hosts only; use the authenticated " - "dashboard /mcp endpoint for remote access" - ) - - -def _transport_security(host: str, port: int): - """Build the SDK's Host/Origin allowlist for the address this launcher binds.""" - from mcp.server.transport_security import TransportSecuritySettings - - address = ipaddress.ip_address(host) - authority = f"[{address.compressed}]" if address.version == 6 else address.compressed - return TransportSecuritySettings( - enable_dns_rebinding_protection=True, - allowed_hosts=[authority, f"{authority}:{port}"], - allowed_origins=[f"http://{authority}", f"http://{authority}:{port}"], - ) - - -def _port(value: str) -> int: - try: - port = int(value) - except ValueError as exc: - raise argparse.ArgumentTypeError("port must be an integer") from exc - if not 1 <= port <= 65535: - raise argparse.ArgumentTypeError("port must be between 1 and 65535") - return port - - -def main(argv=None) -> None: - ap = argparse.ArgumentParser( - prog="engraphis-mcp-http", - description="Run a loopback-only Engraphis MCP server over HTTP.", - epilog=( - "Use the authenticated dashboard /mcp endpoint for remote clients. " - "Configuration also honors ENGRAPHIS_DB_PATH and the normal .env settings." - ), - ) - ap.add_argument( - "--host", - type=_loopback_host, - default=os.environ.get("ENGRAPHIS_HTTP_HOST", "127.0.0.1"), - help="loopback address to bind (default: ENGRAPHIS_HTTP_HOST or 127.0.0.1)", - ) - ap.add_argument( - "--port", - type=_port, - default=os.environ.get("ENGRAPHIS_HTTP_PORT", "8711"), - help="TCP port to bind (default: ENGRAPHIS_HTTP_PORT or 8711)", - ) - ap.add_argument( - "--transport", - choices=_TRANSPORTS, - default=os.environ.get("ENGRAPHIS_HTTP_TRANSPORT", "streamable-http"), - help="MCP transport (default: ENGRAPHIS_HTTP_TRANSPORT or streamable-http)", - ) - ap.add_argument( - "--classic", - action="store_true", - help=( - "serve the legacy 34 direct-tool surface; normal use defaults to the compact " - "Smart gateway" - ), - ) - args = ap.parse_args(argv) - if args.transport not in _TRANSPORTS: - ap.error("ENGRAPHIS_HTTP_TRANSPORT must be streamable-http or sse") - - error = _dependency_error() - if error: - raise SystemExit(error) - - # Import only after --help and dependency validation: FastMCP registers tools at - # module import time, so importing it eagerly would make even help unusable. - from engraphis.mcp_server import mcp - - # The eager exact-backend check may be absent from test mocks that replace - # engraphis.mcp_server with a minimal stand-in; fall back to a no-op so - # those tests stay green while production callers always run the check. - try: - from engraphis.mcp_server import _eager_exact_backend_check - except ImportError: - _eager_exact_backend_check = lambda: None # noqa: E731 - - server = mcp - if args.classic: - from engraphis.mcp_server import classic_mcp - - server = classic_mcp - server.settings.host = args.host - server.settings.port = args.port - server.settings.transport_security = _transport_security(args.host, args.port) - # Restart-resilient transport. FastMCP's default *stateful* mode tracks MCP - # session ids in memory, so every service bounce (pm2 resurrect, watchdog, - # manual restart) invalidates all live session ids: the client's next request - # gets a 404, the mcp SDK raises "Session terminated", and Hermes' gateway - # client parks for its full retry interval with zero registered tools. - # Stateless mode makes each POST self-contained per the MCP spec, so any - # healthy process can answer any request. Spec-compliant clients handle the - # absent GET SSE stream (the server answers 405 and clients skip it). - server.settings.stateless_http = True - _eager_exact_backend_check() - server.run(transport=args.transport) - - -if __name__ == "__main__": - main() +"""Console entry for a local loopback MCP-over-HTTP server. + +This is intentionally a generic MCP transport, not an integration with any particular +agent host. Remote MCP access belongs behind the authenticated dashboard ``/mcp`` +mount; a standalone FastMCP transport has no Engraphis authentication middleware. +""" +from __future__ import annotations + +import argparse +import importlib.util +import ipaddress +import logging +import os +import sys + +_TRANSPORTS = ("streamable-http", "sse") + + +def _configure_logging() -> None: + """Opt-in INFO-level logs for the standalone MCP HTTP launcher. + + The MCP SDK and ``engraphis_recall_context`` use ``logging.getLogger(__name__)``, + which falls back to a ``NullHandler`` when no root config is set. Operators who + want per-call visibility can set ``ENGRAPHIS_MCP_LOG=info`` (or any non-empty + truthy value); the launcher then wires ``logging.basicConfig(level=INFO)`` so + logs reach stderr. Default behaviour is silent to preserve the standalone + CLI's quietness. + """ + if os.environ.get("ENGRAPHIS_MCP_LOG", "").strip().lower() in { + "1", "true", "yes", "info", "on", + }: + if not logging.getLogger().handlers: + logging.basicConfig( + level=logging.INFO, + format="%(asctime)s %(levelname)s %(name)s: %(message)s", + ) + + +def _dependency_error() -> str: + if sys.version_info < (3, 10): + return ( + "The Engraphis MCP server requires Python 3.10 or newer.\n" + "Create a Python 3.10+ environment, then run: pip install \"engraphis[mcp]\"" + ) + if importlib.util.find_spec("mcp") is None: + return ( + "The 'mcp' package is required to run the Engraphis MCP server.\n" + "Install it with: pip install \"engraphis[mcp]\"" + ) + return "" + + +def _loopback_host(value: str) -> str: + host = value.strip() + # Accept 'localhost' as a synonym for 127.0.0.1 - standard network tool behavior. + if host.lower() == "localhost": + return "127.0.0.1" + try: + if ipaddress.ip_address(host).is_loopback: + return host + except ValueError: + pass + raise argparse.ArgumentTypeError( + "standalone MCP-over-HTTP accepts loopback hosts only; use the authenticated " + "dashboard /mcp endpoint for remote access" + ) + + +def _transport_security(host: str, port: int): + """Build the SDK's Host/Origin allowlist for the address this launcher binds.""" + from mcp.server.transport_security import TransportSecuritySettings + + address = ipaddress.ip_address(host) + authority = f"[{address.compressed}]" if address.version == 6 else address.compressed + return TransportSecuritySettings( + enable_dns_rebinding_protection=True, + allowed_hosts=[authority, f"{authority}:{port}"], + allowed_origins=[f"http://{authority}", f"http://{authority}:{port}"], + ) + + +def _port(value: str) -> int: + try: + port = int(value) + except ValueError as exc: + raise argparse.ArgumentTypeError("port must be an integer") from exc + if not 1 <= port <= 65535: + raise argparse.ArgumentTypeError("port must be between 1 and 65535") + return port + + +def main(argv=None) -> None: + ap = argparse.ArgumentParser( + prog="engraphis-mcp-http", + description="Run a loopback-only Engraphis MCP server over HTTP.", + epilog=( + "Use the authenticated dashboard /mcp endpoint for remote clients. " + "Configuration also honors ENGRAPHIS_DB_PATH and the normal .env settings." + ), + ) + ap.add_argument( + "--host", + type=_loopback_host, + default=os.environ.get("ENGRAPHIS_HTTP_HOST", "127.0.0.1"), + help="loopback address to bind (default: ENGRAPHIS_HTTP_HOST or 127.0.0.1)", + ) + ap.add_argument( + "--port", + type=_port, + default=os.environ.get("ENGRAPHIS_HTTP_PORT", "8711"), + help="TCP port to bind (default: ENGRAPHIS_HTTP_PORT or 8711)", + ) + ap.add_argument( + "--transport", + choices=_TRANSPORTS, + default=os.environ.get("ENGRAPHIS_HTTP_TRANSPORT", "streamable-http"), + help="MCP transport (default: ENGRAPHIS_HTTP_TRANSPORT or streamable-http)", + ) + ap.add_argument( + "--classic", + action="store_true", + help=( + "serve the legacy 34 direct-tool surface; normal use defaults to the compact " + "Smart gateway" + ), + ) + args = ap.parse_args(argv) + if args.transport not in _TRANSPORTS: + ap.error("ENGRAPHIS_HTTP_TRANSPORT must be streamable-http or sse") + + error = _dependency_error() + if error: + raise SystemExit(error) + + _configure_logging() + + # Import only after --help and dependency validation: FastMCP registers tools at + # module import time, so importing it eagerly would make even help unusable. + from engraphis.mcp_server import mcp + + # The eager exact-backend check may be absent from test mocks that replace + # engraphis.mcp_server with a minimal stand-in; fall back to a no-op so + # those tests stay green while production callers always run the check. + try: + from engraphis.mcp_server import _eager_exact_backend_check + except ImportError: + _eager_exact_backend_check = lambda: None # noqa: E731 + + server = mcp + if args.classic: + from engraphis.mcp_server import classic_mcp + + server = classic_mcp + server.settings.host = args.host + server.settings.port = args.port + server.settings.transport_security = _transport_security(args.host, args.port) + # Restart-resilient transport. FastMCP's default *stateful* mode tracks MCP + # session ids in memory, so every service bounce (pm2 resurrect, watchdog, + # manual restart) invalidates all live session ids: the client's next request + # gets a 404, the mcp SDK raises "Session terminated", and Hermes' gateway + # client parks for its full retry interval with zero registered tools. + # Stateless mode makes each POST self-contained per the MCP spec, so any + # healthy process can answer any request. Spec-compliant clients handle the + # absent GET SSE stream (the server answers 405 and clients skip it). + server.settings.stateless_http = True + _eager_exact_backend_check() + server.run(transport=args.transport) + + +if __name__ == "__main__": + main() diff --git a/engraphis/mcp_server.py b/engraphis/mcp_server.py index 8e75d6d3..67761e5e 100644 --- a/engraphis/mcp_server.py +++ b/engraphis/mcp_server.py @@ -28,6 +28,7 @@ import json import logging import re +import time import secrets import math @@ -35,11 +36,12 @@ from typing import Any, Annotated, Callable, List, Optional try: - from pydantic import Field, StrictBool, StrictInt + from pydantic import BeforeValidator, Field, StrictBool, StrictInt except ImportError: # pragma: no cover - core-floor (numpy-only) installs Field = None # type: ignore[assignment,misc] StrictBool = None # type: ignore[assignment,misc] StrictInt = None # type: ignore[assignment,misc] + BeforeValidator = None # type: ignore[assignment,misc] try: from mcp.server.fastmcp import FastMCP @@ -673,7 +675,7 @@ def engraphis_recall_context( description="Optional active session; includes its repo/workspace ancestors.")] = None, mtypes: Annotated[Optional[List[str]], Field( description="Optional memory types: semantic/episodic/procedural/working.")] = None, - k: Annotated[int, Field(description="Max candidate memories (1-50).", ge=1, le=50)] = 8, + k: Annotated[int, Field(description="Max candidate memories (1-50).", ge=1, le=50)] = 50, token_budget: Annotated[int, Field( description="Hard packed-context budget under the reported token counter.", ge=0, le=32_768)] = 1024, @@ -708,6 +710,7 @@ def engraphis_recall_context( ``semantic_support`` flags as ``engraphis_recall``. """ try: + _recall_started = time.monotonic() payload = service().recall( query, workspace=workspace, @@ -758,6 +761,17 @@ def engraphis_recall_context( sources.append(source) payload["sources"] = sources payload = _apply_response_budget(payload, max_response_tokens) + usage = payload.get("usage") or {} + # The recall usage dict only carries token and packing counters; latency + # is captured here so the operational log reports the real per-call + # cost of the recall (not a fixed zero from a non-existent field). + elapsed_ms = (time.monotonic() - _recall_started) * 1000.0 + logger.info( + "recall_context workspace=%s k=%s budget=%s packed=%s omitted=%s ms=%.0f", + workspace, k, token_budget, + usage.get("packed_count"), usage.get("omitted_count"), + elapsed_ms, + ) return _ok(payload) except Exception as exc: # noqa: BLE001 return _err(exc) @@ -2533,6 +2547,21 @@ def _record_gateway_execution( log_level="WARNING") +def _normalize_session_action(value: Any) -> Any: + """Tolerate callers that pass the full tool name as the session action. + + The Command Code harness translates AGENTS.md's ``engraphis_start_session`` + shorthand into ``engraphis_session(action="start_session")``; normalize that + to ``start`` (and ``end_session`` to ``end``) before the pattern constraint + is applied, so the call succeeds instead of raising a validation error. + """ + if value == "start_session": + return "start" + if value == "end_session": + return "end" + return value + + @smart_mcp.tool( name="engraphis_session", annotations={"title": "Start or end a memory session", "readOnlyHint": False, @@ -2540,8 +2569,12 @@ def _record_gateway_execution( structured_output=False, ) def engraphis_session( - action: Annotated[str, Field(description="start to resume work, or end to save its handoff.", - pattern="^(start|end)$")] = "start", + action: Annotated[ + str, + BeforeValidator(_normalize_session_action), + Field(description="start to resume work, or end to save its handoff.", + pattern="^(start|end)$"), + ] = "start", workspace: Annotated[str, Field(description="Workspace for a started session.", max_length=200)] = "default", repo: Annotated[Optional[str], Field(description="Optional repository scope.", max_length=200)] = None, agent: Annotated[str, Field(description="Optional agent name.", max_length=200)] = "", @@ -2559,6 +2592,12 @@ def engraphis_session( le=32_768)] = 512, ) -> str: """Start/resume a session or end it with its next-session handoff.""" + # Direct (non-protocol) callers bypass Pydantic's BeforeValidator, so + # normalize the shorthand tool-name forms here too. + if action == "start_session": + action = "start" + elif action == "end_session": + action = "end" if action == "end": if not session_id: return _gateway_error("session_id_required") @@ -2606,7 +2645,7 @@ def smart_recall_context( workspace: Annotated[Optional[str], Field(description="Optional workspace.", max_length=200)] = None, repo: Annotated[Optional[str], Field(description="Optional repository.", max_length=200)] = None, session_id: Annotated[Optional[str], Field(description="Optional active session.")] = None, - k: Annotated[int, Field(description="Maximum source memories.", ge=1, le=50)] = 8, + k: Annotated[int, Field(description="Maximum source memories.", ge=1, le=50)] = 50, token_budget: Annotated[int, Field(description="Hard returned-context token budget.", ge=0, le=32_768)] = 1024, ) -> str: @@ -2635,11 +2674,19 @@ def smart_remember( mtype: Annotated[str, Field(description="semantic, episodic, procedural, or working.")] = "semantic", importance: Annotated[float, Field(description="Salience from 0 to 1.", ge=0.0, le=1.0)] = 0.0, + subject_key: Annotated[str, Field( + description="Optional stable claim subject (for example 'api.rate_limit'). " + "Matching keys make supersession safer and deterministic.", + max_length=1_000)] = "", + claim_kind: Annotated[str, Field( + description="Optional claim predicate/category (for example 'configured_value').", + max_length=200)] = "", ) -> str: """Store a routine durable memory with safe default provenance and deduplication.""" result = engraphis_remember( content=content, workspace=workspace, repo=repo, session_id=session_id, mtype=mtype, importance=importance, + subject_key=subject_key, claim_kind=claim_kind, ) if isinstance(result, str) and result.startswith("Error:"): return _smart_error_from_string(result) diff --git a/eval/datasets/resolver_reworded_corrections.jsonl b/eval/datasets/resolver_reworded_corrections.jsonl new file mode 100644 index 00000000..a48c4388 --- /dev/null +++ b/eval/datasets/resolver_reworded_corrections.jsonl @@ -0,0 +1,44 @@ +{"id": "rc01", "neighbor": "The request timeout is 30 seconds.", "candidate": "We raised the request timeout to 90 seconds last sprint.", "expected": "invalidate", "subject_hint": "request timeout"} +{"id": "rc02", "neighbor": "Deploys run on Fridays at 5pm.", "candidate": "Deploys now run on Tuesdays at 7pm.", "expected": "invalidate", "subject_hint": "deploys"} +{"id": "rc03", "neighbor": "The pilot cohort has 25 users.", "candidate": "The pilot cohort has 120 users now.", "expected": "invalidate", "subject_hint": "pilot cohort"} +{"id": "rc04", "neighbor": "Until January the rate limit was 100 requests per minute.", "candidate": "As of February the rate limit is 500 requests per minute.", "expected": "invalidate", "subject_hint": "rate limit"} +{"id": "rc05", "neighbor": "Database connection pool holds 5 connections per app instance.", "candidate": "Database connection pool holds 8 connections per app instance.", "expected": "invalidate", "subject_hint": "connection pool"} +{"id": "rc06", "neighbor": "The default branch is named master.", "candidate": "The default branch is named main.", "expected": "invalidate", "subject_hint": "default branch"} +{"id": "rc07", "neighbor": "The HTTP port is 8080.", "candidate": "We moved the HTTP port to 9090.", "expected": "invalidate", "subject_hint": "HTTP port"} +{"id": "rc08", "neighbor": "API rate limit is 60 per minute.", "candidate": "API rate limit increased to 120 per minute.", "expected": "invalidate", "subject_hint": "rate limit"} +{"id": "rc09", "neighbor": "The cluster runs 3 replicas.", "candidate": "The cluster now runs 5 replicas.", "expected": "invalidate", "subject_hint": "cluster replicas"} +{"id": "rc10", "neighbor": "The default admin user is root.", "candidate": "The default admin user is now admin.", "expected": "invalidate", "subject_hint": "admin user"} +{"id": "rc11", "neighbor": "Cache TTL is 300 seconds.", "candidate": "Cache TTL is 600 seconds.", "expected": "invalidate", "subject_hint": "cache ttl"} +{"id": "rc12", "neighbor": "The build takes 4 minutes.", "candidate": "The build takes 7 minutes now.", "expected": "invalidate", "subject_hint": "build time"} +{"id": "rc13", "neighbor": "Max upload size is 10MB.", "candidate": "Max upload size is 50MB.", "expected": "invalidate", "subject_hint": "upload size"} +{"id": "rc14", "neighbor": "Workers run on port 5000.", "candidate": "Workers run on port 6000.", "expected": "invalidate", "subject_hint": "worker port"} +{"id": "rc15", "neighbor": "The SLA is 99.9 percent.", "candidate": "The SLA is 99.99 percent now.", "expected": "invalidate", "subject_hint": "SLA"} +{"id": "rc16", "neighbor": "Log retention is 7 days.", "candidate": "Log retention is 30 days.", "expected": "invalidate", "subject_hint": "log retention"} +{"id": "rc17", "neighbor": "The backup window is 02:00 UTC.", "candidate": "The backup window is 04:00 UTC.", "expected": "invalidate", "subject_hint": "backup window"} +{"id": "rc18", "neighbor": "The S3 bucket holds 100GB.", "candidate": "The S3 bucket now holds 500GB.", "expected": "invalidate", "subject_hint": "S3 bucket"} +{"id": "rc19", "neighbor": "The CDN serves from us-east-1.", "candidate": "The CDN serves from us-west-2.", "expected": "invalidate", "subject_hint": "CDN region"} +{"id": "rc20", "neighbor": "The poll interval is 5 seconds.", "candidate": "The poll interval is 10 seconds.", "expected": "invalidate", "subject_hint": "poll interval"} +{"id": "rc21", "neighbor": "Default page size is 20.", "candidate": "Default page size is 50.", "expected": "invalidate", "subject_hint": "page size"} +{"id": "rc22", "neighbor": "The search timeout is 3 seconds.", "candidate": "The search timeout is 5 seconds now.", "expected": "invalidate", "subject_hint": "search timeout"} +{"id": "rc23", "neighbor": "The max payload is 1MB.", "candidate": "The max payload is 5MB.", "expected": "invalidate", "subject_hint": "max payload"} +{"id": "rc24", "neighbor": "The retry count is 3.", "candidate": "The retry count is 5 now.", "expected": "invalidate", "subject_hint": "retry count"} +{"id": "rc25", "neighbor": "The session timeout is 30 minutes.", "candidate": "The session timeout is 60 minutes.", "expected": "invalidate", "subject_hint": "session timeout"} +{"id": "rc26", "neighbor": "The deployment runs in 2 minutes.", "candidate": "The deployment runs in 5 minutes now.", "expected": "invalidate", "subject_hint": "deployment time"} +{"id": "rc27", "neighbor": "The queue holds 100 messages.", "candidate": "The queue holds 1000 messages.", "expected": "invalidate", "subject_hint": "queue size"} +{"id": "rc28", "neighbor": "The circuit breaker trips at 5 errors.", "candidate": "The circuit breaker trips at 10 errors.", "expected": "invalidate", "subject_hint": "circuit breaker"} +{"id": "rc29", "neighbor": "Connection pool is 10 per host.", "candidate": "Connection pool is 20 per host.", "expected": "invalidate", "subject_hint": "connection pool per host"} +{"id": "rc30", "neighbor": "Default branch protection requires 1 review.", "candidate": "Default branch protection requires 2 reviews.", "expected": "invalidate", "subject_hint": "branch protection"} +{"id": "rc31", "neighbor": "Job timeout is 1 hour.", "candidate": "Job timeout is 4 hours now.", "expected": "invalidate", "subject_hint": "job timeout"} +{"id": "rc32", "neighbor": "The retry backoff is 1 second.", "candidate": "The retry backoff is 5 seconds.", "expected": "invalidate", "subject_hint": "retry backoff"} +{"id": "rc33", "neighbor": "Default log level is INFO.", "candidate": "Default log level is DEBUG now.", "expected": "invalidate", "subject_hint": "log level"} +{"id": "rc34", "neighbor": "The user agent is engraphis/1.0.", "candidate": "The user agent is engraphis/2.0.", "expected": "invalidate", "subject_hint": "user agent"} +{"id": "rc35", "neighbor": "The webhook secret rotates every 30 days.", "candidate": "The webhook secret rotates every 90 days.", "expected": "invalidate", "subject_hint": "webhook secret"} +{"id": "rc36", "neighbor": "API tokens expire after 24 hours.", "candidate": "API tokens expire after 7 days now.", "expected": "invalidate", "subject_hint": "API token expiry"} +{"id": "df01", "neighbor": "The production API uses Redis caching for user sessions.", "candidate": "The production API now uses three replicas for high availability.", "expected": "add", "subject_hint": "API infra (different facts)"} +{"id": "df02", "neighbor": "The docs cover the REST interface.", "candidate": "We migrated the docs to cover the GraphQL interface.", "expected": "add", "subject_hint": "docs interface (different facts)"} +{"id": "df03", "neighbor": "CI runs on ProviderA with 4 workers.", "candidate": "We switched CI to run on ProviderB with 8 workers.", "expected": "add", "subject_hint": "CI infra (different facts)"} +{"id": "df04", "neighbor": "The staging database holds 300 connections in production environment.", "candidate": "The production database holds 300 connections in staging environment.", "expected": "add", "subject_hint": "staging/production (env conflict)"} +{"id": "df05", "neighbor": "Production API timeout is 30 seconds.", "candidate": "Production API timeout increased to 90 seconds.", "expected": "invalidate", "subject_hint": "API timeout (value swap)"} +{"id": "df06", "neighbor": "Prod API timeout is 30 seconds.", "candidate": "Production API timeout increased to 90 seconds.", "expected": "invalidate", "subject_hint": "env alias prod==production"} +{"id": "df07", "neighbor": "The staging database pool is 5 connections.", "candidate": "The development database pool is 5 connections.", "expected": "add", "subject_hint": "staging vs dev (env conflict)"} +{"id": "df08", "neighbor": "The primary database pool in staging holds 5 connections per app instance.", "candidate": "The primary database pool in production holds 8 connections per app instance.", "expected": "add", "subject_hint": "staging vs production (env conflict)"} diff --git a/eval/resolver_reworded_corrections.py b/eval/resolver_reworded_corrections.py new file mode 100644 index 00000000..42391f0a --- /dev/null +++ b/eval/resolver_reworded_corrections.py @@ -0,0 +1,152 @@ +"""Reproducible eval for the reworded-correction resolver. + +Drives ``engraphis.core.resolve.resolve()`` over a labeled JSONL corpus +and reports the number of true-positives superseded and the number of +false-positives (distinct facts the resolver incorrectly merged). The +corpus and this script together protect the quality claim made in +``CHANGELOG.md``; rerun with: + + python -m eval.resolver_reworded_corrections + +The dataset ships at ``eval/datasets/resolver_reworded_corrections.jsonl`` +and contains 36 positive (reworded-correction) pairs and 8 negative +(distinct-fact / env-conflict) pairs. Each row is:: + + {"id", "neighbor", "candidate", "expected", "subject_hint"} + +``expected`` is one of ``"invalidate"`` (the resolver should mark the +candidate as a correction of the neighbour) or ``"add"`` (the resolver +should add it as a distinct fact). + +This is an offline-only evaluation: it does not require an embedder, +``engraphis-mcp``, or any external service. The resolver's only +configuration is its importable constants. +""" +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path +from typing import Any + +from engraphis.core.interfaces import MemoryRecord +from engraphis.core.resolve import resolve + +DATASET = Path(__file__).resolve().parent / "datasets" / "resolver_reworded_corrections.jsonl" + + +def _memory_record(text: str, record_id: str) -> MemoryRecord: + return MemoryRecord( + id=record_id, workspace_id="w", repo_id=None, session_id=None, + title="", content=text, mtype="semantic", scope="workspace", + importance=0.0, confidence=1.0, valid_from=0.0, valid_to=None, + ingested_at=0.0, expired_at=None, + subject_key="", claim_kind="", keywords=(), metadata={}, + ) + + +def evaluate(dataset: Path = DATASET) -> dict[str, Any]: + positives = 0 + positives_superseded = 0 + negatives = 0 + false_invalidations: list[dict[str, Any]] = [] + missed_corrections: list[dict[str, Any]] = [] + total = 0 + with dataset.open("r", encoding="utf-8") as f: + for line in f: + line = line.strip() + if not line: + continue + row = json.loads(line) + total += 1 + expected = row["expected"] + neighbor = _memory_record(row["neighbor"], f"mem_{row['id']}_n") + # Use a high similarity so the resolver's strong/rewrite gates + # are exercised for every row. The labeled ground truth tells + # us whether the resolver should INVALIDATE or ADD. + resolution = resolve( + row["candidate"], + [(0.9, neighbor)], + ) + actual = resolution.op.value + if expected == "invalidate": + positives += 1 + if actual == "invalidate": + positives_superseded += 1 + else: + missed_corrections.append({ + "id": row["id"], + "expected": "invalidate", + "actual": actual, + "reason": resolution.reason, + "subject_hint": row.get("subject_hint", ""), + }) + else: + negatives += 1 + if actual == "invalidate": + false_invalidations.append({ + "id": row["id"], + "expected": "add", + "actual": actual, + "reason": resolution.reason, + "subject_hint": row.get("subject_hint", ""), + }) + summary = { + "dataset": str(dataset), + "total": total, + "positives": positives, + "negatives": negatives, + "positives_superseded": positives_superseded, + "false_invalidations": len(false_invalidations), + "missed_corrections": len(missed_corrections), + "missed_correction_ids": [m["id"] for m in missed_corrections], + "false_invalidation_ids": [f["id"] for f in false_invalidations], + } + return summary + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__.split("\n\n", 1)[0]) + parser.add_argument( + "--dataset", + type=Path, + default=DATASET, + help="Path to the labeled JSONL corpus (default: %(default)s).", + ) + parser.add_argument( + "--strict", + action="store_true", + help="Exit non-zero if any positive is missed or any negative is " + "false-invalidated. The default is to report and exit 0 so this " + "script can be run in CI as an audit log without flaking on " + "regressions; use --strict to gate the build.", + ) + args = parser.parse_args(argv) + + if not args.dataset.exists(): + print(f"error: dataset not found at {args.dataset}", file=sys.stderr) + return 2 + + summary = evaluate(args.dataset) + positives = summary["positives"] + superseded = summary["positives_superseded"] + negatives = summary["negatives"] + false_inv = summary["false_invalidations"] + print( + f"resolver reworded-correction eval: " + f"{superseded}/{positives} positives superseded, " + f"{false_inv}/{negatives} false invalidations, " + f"{summary['total']} pairs total" + ) + if summary["missed_correction_ids"]: + print(f" missed corrections: {summary['missed_correction_ids']}") + if summary["false_invalidation_ids"]: + print(f" false invalidations: {summary['false_invalidation_ids']}") + if args.strict and (superseded < positives or false_inv > 0): + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/integrations/commandcode/session_start_hook.py b/integrations/commandcode/session_start_hook.py new file mode 100644 index 00000000..734686f9 --- /dev/null +++ b/integrations/commandcode/session_start_hook.py @@ -0,0 +1,192 @@ +"""Command Code SessionStart hook for Engraphis durable memory. + +Reads the SessionStart payload from stdin, opens a session against the local +Engraphis MCP server, and emits hook JSON whose ``additionalContext`` carries +bounded recalled memory into the new session's first turn. Fails open: any +error, timeout, or empty context prints nothing and exits 0. +""" + +import json +import os +import sys +import time +import urllib.request + +MCP_URL = os.environ.get("ENGRAPHIS_MCP_URL", "http://127.0.0.1:8711/mcp") +BUDGET_SECONDS = float(os.environ.get("ENGRAPHIS_HOOK_BUDGET_S", "4.0")) +MAX_CONTEXT_CHARS = int(os.environ.get("ENGRAPHIS_HOOK_MAX_CHARS", "1500")) +CONTEXT_HEADER = ( + "Durable memory (engraphis, workspace {workspace}) relevant to this repo:\n" +) +CONTEXT_FOOTER = "\nUse mcp__engraphis__ tools for more recall." + +# Localhost server; never route through a configured system proxy. +OPENER = urllib.request.build_opener(urllib.request.ProxyHandler({})) + + +def post(url, payload, timeout): + """POST one JSON-RPC message; return its decoded JSON or SSE response.""" + request = urllib.request.Request( + url, + data=json.dumps(payload).encode("utf-8"), + headers={ + "Content-Type": "application/json", + "Accept": "application/json, text/event-stream", + }, + method="POST", + ) + with OPENER.open(request, timeout=timeout) as response: + body = response.read().decode("utf-8", errors="replace") + try: + return json.loads(body) + except ValueError: + pass + candidates = [] + for line in body.splitlines(): + line = line.strip() + if not line.startswith("data:"): + continue + try: + candidates.append(json.loads(line[5:].strip())) + except ValueError: + continue + responses = [c for c in candidates if isinstance(c, dict) and "result" in c] + return responses[-1] if responses else None + + +def rpc(method, params, rpc_id, deadline): + """Issue one JSON-RPC request within the shared time budget.""" + remaining = deadline - time.monotonic() + if remaining <= 0.05: + raise TimeoutError("time budget exhausted") + response = post(MCP_URL, {"jsonrpc": "2.0", "id": rpc_id, "method": method, "params": params}, remaining) + if isinstance(response, dict) and "result" in response: + return response["result"] + return None + + +def notify_initialized(deadline): + """Best-effort notifications/initialized; stateless servers reply 202/empty.""" + remaining = deadline - time.monotonic() + if remaining <= 0.05: + return + try: + post(MCP_URL, {"jsonrpc": "2.0", "method": "notifications/initialized"}, remaining) + except Exception: + pass + + +def extract_context(result): + """Defensively pull the context text out of a tools/call result.""" + content = result.get("content") if isinstance(result, dict) else None + if not content or not isinstance(content[0], dict): + return "" + text = content[0].get("text") + if not isinstance(text, str) or not text.strip(): + return "" + try: + parsed = json.loads(text) + except ValueError: + return text.strip() + if isinstance(parsed, dict) and isinstance(parsed.get("context"), str): + return parsed["context"].strip() + return "" + + +def session_context(repo, workspace, deadline): + """initialize -> initialized -> tools/call engraphis_session(action=start).""" + rpc( + "initialize", + { + "protocolVersion": "2025-03-26", + "capabilities": {}, + "clientInfo": {"name": "cc-hook", "version": "1.0"}, + }, + 1, + deadline, + ) + notify_initialized(deadline) + result = rpc( + "tools/call", + { + "name": "engraphis_session", + "arguments": { + "action": "start", + "workspace": workspace, + "repo": repo, + # Context is only returned when a goal is supplied. + "goal": ( + "Resume work on this repository: surface relevant durable " + "decisions, preferences, procedures, and open threads." + ), + }, + }, + 2, + deadline, + ) + return extract_context(result) + + +def resolve_workspace(cwd, env): + """Honor ENGRAPHIS_HOOK_WORKSPACE; otherwise fall back to the repo basename. + + Workspace names are bounded (``_clean_name`` in the service refuses empties and + overlong inputs), so we drop the override silently if it would be rejected. + """ + override = (env.get("ENGRAPHIS_HOOK_WORKSPACE") or "").strip() + if override: + return override + return os.path.basename(os.path.normpath(str(cwd))) + + +def build_additional_context(context, workspace): + header = CONTEXT_HEADER.format(workspace=workspace) + footer = CONTEXT_FOOTER + body_budget = MAX_CONTEXT_CHARS - len(header) - len(footer) + if body_budget <= 0: + # Header+footer already exceed the budget. Truncate the header so the + # final payload stays within MAX_CONTEXT_CHARS and the agent still gets + # a recognisable prompt header for the workspace. + return (header + footer)[:MAX_CONTEXT_CHARS] + return (header + context[:body_budget] + footer)[:MAX_CONTEXT_CHARS] + + +def main(): + deadline = time.monotonic() + BUDGET_SECONDS + try: + payload = json.loads(sys.stdin.read() or "{}") + except Exception: + return 0 + if not isinstance(payload, dict): + return 0 + name = payload.get("hook_event_name") + if name is not None and name != "SessionStart": + return 0 + cwd = payload.get("cwd") or os.environ.get("COMMANDCODE_PROJECT_DIR") or os.getcwd() + repo = os.path.basename(os.path.normpath(str(cwd))) + workspace = resolve_workspace(cwd, os.environ) + try: + context = session_context(repo, workspace, deadline) + except Exception: + return 0 + if not context: + return 0 + output = { + "suppressOutput": False, + "hookSpecificOutput": { + "hookEventName": "SessionStart", + "additionalContext": build_additional_context(context, workspace), + }, + } + sys.stdout.write(json.dumps(output)) + sys.stdout.flush() + return 0 + + +if __name__ == "__main__": + try: + sys.exit(main()) + except SystemExit: + raise + except BaseException: + sys.exit(0) diff --git a/scripts/install_cc_hook.py b/scripts/install_cc_hook.py new file mode 100644 index 00000000..d7c4c822 --- /dev/null +++ b/scripts/install_cc_hook.py @@ -0,0 +1,179 @@ +# -*- coding: utf-8 -*- +"""Install the Engraphis Command Code SessionStart hook into the user-scope settings file. + +Idempotent: running twice updates the existing entry rather than duplicating it. +Backup is written to ``.bak-engraphis-`` on first write only. + +Usage: + python scripts/install_cc_hook.py + python scripts/install_cc_hook.py --uninstall +""" +from __future__ import annotations + +import argparse +import datetime +import json +import os +import sys +from pathlib import Path + +SETTINGS_PATH = Path(os.environ.get("COMMANDCODE_SETTINGS_PATH") + or Path.home() / ".commandcode" / "settings.json") +HOOK_PATH = Path(__file__).resolve().parent.parent / "integrations" / "commandcode" / "session_start_hook.py" +HOOK_KEY = "cc-engraphis-session-start" + + +def _utc_stamp() -> str: + return datetime.datetime.now(datetime.timezone.utc).strftime("%Y%m%d") + + +def _backup(settings_path: Path) -> Path | None: + if not settings_path.exists(): + return None + backup = settings_path.with_name( + f"{settings_path.name}.bak-engraphis-{_utc_stamp()}") + if backup.exists(): + return backup + backup.write_bytes(settings_path.read_bytes()) + return backup + + +def _read_settings(settings_path: Path) -> dict: + if not settings_path.exists(): + return {} + try: + return json.loads(settings_path.read_text(encoding="utf-8") or "{}") + except json.JSONDecodeError as exc: + print(f"error: {settings_path} is not valid JSON: {exc}", file=sys.stderr) + sys.exit(2) + + +def _write_settings(settings_path: Path, settings: dict) -> None: + settings_path.parent.mkdir(parents=True, exist_ok=True) + settings_path.write_text( + json.dumps(settings, indent=2, sort_keys=True) + "\n", + encoding="utf-8", + ) + + +def _hook_entry() -> dict: + return { + "type": "command", + "command": f"python \"{HOOK_PATH}\"", + "name": HOOK_KEY, + "timeout": 5, + } + + +def _entry_command() -> str: + return _hook_entry()["command"] + + +def _is_our_entry(entry: dict) -> bool: + """Match by stable ``name`` (HOOK_KEY) first, then fall back to command string. + + The command path can vary across installations, so a stable identifier is the + primary key; the command match stays as a backstop for legacy entries written + by older versions of this script. + """ + if entry.get("name") == HOOK_KEY: + return True + return entry.get("command", "") == _entry_command() + + +def _session_start_has_our_entry(hooks: list) -> bool: + """Each SessionStart entry is ``{"hooks": [{"command": ...}, ...]}``.""" + for wrapper in hooks: + for entry in wrapper.get("hooks", []) or []: + if _is_our_entry(entry): + return True + return False + + +def _strip_our_entries(wrapper: dict) -> dict | None: + """Return a new wrapper with our inner entries removed. + + Returns ``None`` if the wrapper becomes empty after stripping (caller drops + it). Preserves every sibling inner entry the operator added manually. + """ + remaining = [ + entry for entry in wrapper.get("hooks", []) or [] + if not _is_our_entry(entry) + ] + if not remaining: + return None + return {"hooks": remaining} + + +def _refresh_existing_wrappers(hooks: list) -> bool: + """Replace any of our inner entries in-place inside matching wrappers. + + Returns ``True`` if at least one wrapper already contained our entry (so the + caller knows a fresh append is not required). Sibling inner entries are + preserved; a wrapper that contained only our entry is replaced with the + fresh entry instead of being kept as an empty wrapper. + """ + refreshed = False + for i, wrapper in enumerate(hooks): + inner = wrapper.get("hooks", []) or [] + if not any(_is_our_entry(e) for e in inner): + continue + refreshed = True + siblings = [e for e in inner if not _is_our_entry(e)] + # Re-add the fresh entry alongside the siblings so the original + # wrapper is preserved verbatim except for our entry being replaced. + hooks[i] = {"hooks": [*siblings, _hook_entry()]} + return refreshed + + +def install() -> None: + settings = _read_settings(SETTINGS_PATH) + hooks = settings.setdefault("hooks", {}).setdefault("SessionStart", []) + # Idempotency: refresh our entry in-place inside any wrapper that already + # contains it, so a manually-added sibling inner hook is preserved and we + # do not append a second wrapper. Only fall through to a fresh append when + # no prior wrapper mentions us. + if not _refresh_existing_wrappers(hooks): + hooks.append({"hooks": [_hook_entry()]}) + _backup(SETTINGS_PATH) + _write_settings(SETTINGS_PATH, settings) + print(f"installed SessionStart hook into {SETTINGS_PATH}") + + +def uninstall() -> None: + settings = _read_settings(SETTINGS_PATH) + if "hooks" not in settings or "SessionStart" not in settings["hooks"]: + print(f"no SessionStart hook entry in {SETTINGS_PATH}") + return + cleaned: list = [] + for wrapper in settings["hooks"]["SessionStart"]: + stripped = _strip_our_entries(wrapper) + if stripped is not None: + cleaned.append(stripped) + settings["hooks"]["SessionStart"] = cleaned + if not settings["hooks"]["SessionStart"]: + del settings["hooks"]["SessionStart"] + if not settings["hooks"]: + del settings["hooks"] + _backup(SETTINGS_PATH) + _write_settings(SETTINGS_PATH, settings) + print(f"removed SessionStart hook from {SETTINGS_PATH}") + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__.split("\n\n", 1)[0]) + parser.add_argument("--uninstall", action="store_true", + help="Remove the Engraphis SessionStart hook from the user settings.") + args = parser.parse_args() + if not HOOK_PATH.exists(): + print(f"error: hook script not found at {HOOK_PATH}", file=sys.stderr) + return 2 + if args.uninstall: + uninstall() + else: + install() + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/skills/engraphis-memory/references/TOOLS.md b/skills/engraphis-memory/references/TOOLS.md index ef42a8b3..ac0c3203 100644 --- a/skills/engraphis-memory/references/TOOLS.md +++ b/skills/engraphis-memory/references/TOOLS.md @@ -96,7 +96,7 @@ Return one hard-budget packed context plus compact source identities, without re bodies already represented in `context`. - `query (str)`; `workspace (str, None)`; `repo (str, None)`; `session_id (str, None)`; - `mtypes (list[str], None)`; `k (int, 8)`. + `mtypes (list[str], None)`; `k (int, 50)`. - `token_budget (int, 1024)`: hard packed-context budget, `0..32768`. - `retrieval_profile (str, "balanced")`: `balanced` is the default legacy hybrid; `auto` is explicit opt-in, with `fast`, `lexical`, `graph`, and `code` available for deliberate routing. The @@ -471,8 +471,8 @@ or mismatched schemas and enforce the declared side-effect boundary. The two overlapping names deliberately have smaller Smart schemas than their Classic sections above. Smart `engraphis_remember` accepts only `content`, `workspace`, `repo`, `session_id`, -`mtype`, and `importance`; safe provenance and deduplication are fixed internally. Smart -`engraphis_recall_context` accepts only `query`, `workspace`, `repo`, `session_id`, `k`, and +`mtype`, `importance`, `subject_key`, and `claim_kind`; safe provenance is fixed internally. +Smart `engraphis_recall_context` accepts only `query`, `workspace`, `repo`, `session_id`, `k`, and `token_budget`; advanced planning/profile controls are discoverable rather than routine. ### `engraphis_session` diff --git a/tests/test_backends_factories.py b/tests/test_backends_factories.py index b9829413..84061e69 100644 --- a/tests/test_backends_factories.py +++ b/tests/test_backends_factories.py @@ -362,6 +362,45 @@ def unavailable(*args, **kwargs): assert "private/reranker" not in caplog.text +def test_configured_rerank_model_selects_cross_encoder_for_the_engine(monkeypatch): + """The documented ENGRAPHIS_RERANK_MODEL knob must reach the composed engine. + + End-to-end selection proof: MemoryEngine.create(rerank_model=...) composes a live + CrossEncoderReranker (heavy loader stubbed so the offline suite never downloads), + while an unset selector keeps IdentityReranker (covered above). Retrieval quality + of that selection is measured separately by eval.harness runs on the bundled gates. + """ + import engraphis.backends.reranker as reranker_module + from engraphis.core.engine import MemoryEngine + + captured = {} + + class _Model: + def __init__(self, name, **kwargs): + captured.update(name=name, **kwargs) + + def predict(self, pairs, batch_size=32): + return [0.0 for _ in pairs] + + monkeypatch.setitem( + sys.modules, + "sentence_transformers", + SimpleNamespace(CrossEncoder=_Model), + ) + monkeypatch.delenv("ENGRAPHIS_REQUIRE_IMMUTABLE_MODELS", raising=False) + + engine = MemoryEngine.create( + ":memory:", + rerank_model="cross-encoder/ms-marco-MiniLM-L-6-v2", + ) + + assert isinstance(engine.reranker, reranker_module.CrossEncoderReranker) + assert captured == { + "name": "cross-encoder/ms-marco-MiniLM-L-6-v2", + "trust_remote_code": False, + } + + def test_memory_service_forwards_model_provenance_to_the_engine(monkeypatch): import engraphis.service as service_module diff --git a/tests/test_engine.py b/tests/test_engine.py index 1ae282b1..b1c1f37b 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -1293,18 +1293,25 @@ def test_titled_keyed_claim_duplicate_is_a_noop_in_temporal_predecessor_path(): def test_anchored_unkeyed_resolution_keeps_a_closed_historical_predecessor(): + # Bi-temporal splice of a KNOWN-ABOUT series: a ``subject_key`` ties the + # three values into a single chain and the engine marks the temporal + # splice explicitly. Without a key the present-time veto contract applies + # (see ``test_anchored_unkeyed_present_time_stays_live`` below if/when added). eng = MemoryEngine.create(":memory:") wid = eng.store.get_or_create_workspace("w") first = eng.remember_with_resolution( "The deployment rollout phase is alpha.", workspace_id=wid, + subject_key="deploy.phase", claim_kind="stage", valid_from=1_000.0, ) eng.remember_with_resolution( "The deployment rollout phase is gamma.", workspace_id=wid, + subject_key="deploy.phase", claim_kind="stage", valid_from=3_000.0, ) backfilled = eng.remember_with_resolution( "The deployment rollout phase is beta.", workspace_id=wid, + subject_key="deploy.phase", claim_kind="stage", valid_from=2_000.0, ) @@ -1313,6 +1320,24 @@ def test_anchored_unkeyed_resolution_keeps_a_closed_historical_predecessor(): assert eng.store.get_memory(first["id"]).valid_to == 2_000.0 +def test_anchored_unkeyed_present_time_stays_live(): + # Pair to the splice test above: a deliberate ``valid_from`` without a + # ``subject_key`` is a scheduled-future write, not a bi-temporal splice, + # and must stay on the present-time veto contract (heavy/proper/env + # swaps both live). + eng = MemoryEngine.create(":memory:") + wid = eng.store.get_or_create_workspace("w") + eng.remember_with_resolution( + "The deployment rollout phase is alpha.", workspace_id=wid, + valid_from=1_000.0, + ) + future = eng.remember_with_resolution( + "The deployment rollout phase is gamma.", workspace_id=wid, + valid_from=3_000.0, + ) + assert future["op"] in ("add", "relate") + + def test_recall_proactive_includes_last_session_handoff(): eng = MemoryEngine.create(":memory:") wid = eng.store.get_or_create_workspace("w") diff --git a/tests/test_install_cc_hook.py b/tests/test_install_cc_hook.py new file mode 100644 index 00000000..c74fddae --- /dev/null +++ b/tests/test_install_cc_hook.py @@ -0,0 +1,183 @@ +"""Tests for scripts/install_cc_hook.py idempotency and nested-shape handling. + +The SessionStart settings file stores each entry as +``{"hooks": [{"command": "..."}, ...]}``. A naive top-level +``h.get("command", ...)`` filter misses our entry on the inner dict and +double-installs, causing every session start to perform duplicate MCP +recalls. +""" +from __future__ import annotations + +import importlib.util +import json +from pathlib import Path + +import pytest + + +REPO_ROOT = Path(__file__).resolve().parent.parent +SCRIPT_PATH = REPO_ROOT / "scripts" / "install_cc_hook.py" + + +def _load_module(): + spec = importlib.util.spec_from_file_location("install_cc_hook_under_test", SCRIPT_PATH) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + # ``spec.loader`` is typed as the abstract ``Loader`` base; the concrete + # file/source loaders we get here all implement ``exec_module``. + loader = spec.loader + exec_module = getattr(loader, "exec_module") + exec_module(module) + return module + + +@pytest.fixture +def fake_settings(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + """Isolated settings file and module-level env override.""" + settings_path = tmp_path / "settings.json" + monkeypatch.setenv("COMMANDCODE_SETTINGS_PATH", str(settings_path)) + module = _load_module() + return module, settings_path + + +def test_install_appends_one_entry_on_first_run(fake_settings) -> None: + module, settings_path = fake_settings + module.install() + payload = json.loads(settings_path.read_text(encoding="utf-8")) + entries = payload["hooks"]["SessionStart"] + assert len(entries) == 1 + assert entries[0]["hooks"][0]["command"] == module._hook_entry()["command"] + + +def test_install_is_idempotent_on_repeat_runs(fake_settings) -> None: + """Running install() twice must not duplicate the SessionStart entry.""" + module, settings_path = fake_settings + module.install() + module.install() + payload = json.loads(settings_path.read_text(encoding="utf-8")) + entries = payload["hooks"]["SessionStart"] + assert len(entries) == 1 + assert entries[0]["hooks"][0]["command"] == module._hook_entry()["command"] + + +def test_install_does_not_disturb_other_session_start_entries(fake_settings) -> None: + module, settings_path = fake_settings + settings_path.write_text( + json.dumps( + { + "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "some-other-tool --flag", + "timeout": 5, + } + ] + } + ] + } + } + ), + encoding="utf-8", + ) + module.install() + payload = json.loads(settings_path.read_text(encoding="utf-8")) + entries = payload["hooks"]["SessionStart"] + assert len(entries) == 2 + commands = [entry["hooks"][0]["command"] for entry in entries] + assert "some-other-tool --flag" in commands + assert module._hook_entry()["command"] in commands + + +def test_uninstall_removes_only_our_entry(fake_settings) -> None: + module, settings_path = fake_settings + settings_path.write_text( + json.dumps( + { + "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "some-other-tool --flag", + "timeout": 5, + } + ] + }, + {"hooks": [module._hook_entry()]}, + ] + } + } + ), + encoding="utf-8", + ) + module.uninstall() + payload = json.loads(settings_path.read_text(encoding="utf-8")) + entries = payload["hooks"]["SessionStart"] + assert len(entries) == 1 + assert entries[0]["hooks"][0]["command"] == "some-other-tool --flag" + + +def test_install_preserves_sibling_hook_in_same_wrapper(fake_settings) -> None: + """A SessionStart wrapper that contains both our entry and a manually + added sibling inner hook must keep the sibling after a reinstall. The + old behaviour dropped the whole wrapper, silently deleting the + operator's unrelated hook. + """ + module, settings_path = fake_settings + sibling = { + "type": "command", + "command": "some-other-tool --flag", + "timeout": 5, + } + settings_path.write_text( + json.dumps( + { + "hooks": { + "SessionStart": [ + {"hooks": [sibling, module._hook_entry()]}, + ] + } + } + ), + encoding="utf-8", + ) + module.install() + payload = json.loads(settings_path.read_text(encoding="utf-8")) + entries = payload["hooks"]["SessionStart"] + # The original wrapper survives, now with [sibling, fresh_engraphis]. + assert len(entries) == 1 + inner = entries[0]["hooks"] + assert len(inner) == 2 + assert inner[0]["command"] == "some-other-tool --flag" + assert inner[1]["command"] == module._hook_entry()["command"] + + +def test_uninstall_preserves_sibling_hook_in_same_wrapper(fake_settings) -> None: + """uninstall() must not drop a sibling inner hook when stripping ours.""" + module, settings_path = fake_settings + sibling = { + "type": "command", + "command": "some-other-tool --flag", + "timeout": 5, + } + settings_path.write_text( + json.dumps( + { + "hooks": { + "SessionStart": [ + {"hooks": [sibling, module._hook_entry()]}, + ] + } + } + ), + encoding="utf-8", + ) + module.uninstall() + payload = json.loads(settings_path.read_text(encoding="utf-8")) + entries = payload["hooks"]["SessionStart"] + assert len(entries) == 1 + assert entries[0]["hooks"] == [sibling] diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index fa7f2710..b57c0397 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -503,9 +503,18 @@ def test_server_identity_and_tools_registered(): assert classic["engraphis_recall_context"].inputSchema["properties"][ "token_budget" ]["default"] == 1024 + # The candidate pool must be wide enough that the token budget (not the pool + # size) binds by default — otherwise context-packing savings stay ~0%. + assert classic["engraphis_recall_context"].inputSchema["properties"][ + "k" + ]["default"] == 50 assert {"planning", "mtype_limits"} <= set( classic["engraphis_recall_context"].inputSchema.get("properties", {}) ) + smart = {t.name: t for t in asyncio.run(srv.mcp.list_tools())} + assert smart["engraphis_recall_context"].inputSchema["properties"][ + "k" + ]["default"] == 50 assert "as_of" in classic["engraphis_recall_grounded"].inputSchema.get("properties", {}) assert {"valid_at", "known_at", "token_budget", "retrieval_profile", "candidate_depth", "response_mode", "planning", "mtype_limits"} <= set( diff --git a/tests/test_resolve.py b/tests/test_resolve.py index 3b3963c2..cbf8318e 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -296,3 +296,199 @@ def test_resolve_moderate_cosine_low_overlap_still_adds(): "stock decrement.") res = resolve(candidate, [(0.6, neighbor)]) assert res.op == ResolutionOp.ADD + + +# ── reworded corrections without a claim key (benchmark cc0825) ──────────────── + +def test_reworded_number_correction_invalidates_without_claim_key(): + # Same fact, changed value, reworded prose: the aligned diff finds the + # 30 -> 90 swap anchored by the shared "timeout/seconds" attribute. + neighbor = _rec("The request timeout is 30 seconds.", id="mem_old_timeout") + res = resolve("We raised the request timeout to 90 seconds last sprint.", + [(0.5, neighbor)]) + assert res.op == ResolutionOp.INVALIDATE + assert res.target_id == "mem_old_timeout" + + +def test_reworded_marker_correction_invalidates_across_phrasings(): + # A bare change marker ("moved", "grew") is not sufficient on its own + # — common words leak into every sentence. The marker leg now requires + # the candidate to also exhibit a value_swap on the same shared + # subject, so the rewrite is "same fact, new value" rather than a + # different fact about a similar topic. + neighbor = _rec("Deploy schedule runs at 5pm on Fridays.", id="mem_deploy_slot") + res = resolve("Deploy schedule now runs at 6pm on Fridays.", [(0.6, neighbor)]) + assert res.op == ResolutionOp.INVALIDATE + assert res.target_id == "mem_deploy_slot" + + neighbor2 = _rec("The pilot cohort has 25 users.", id="mem_pilot") + res2 = resolve("The pilot cohort has 120 users.", [(0.5, neighbor2)]) + # No marker, but a clear value swap on the same subject. + assert res2.op == ResolutionOp.INVALIDATE + assert res2.target_id == "mem_pilot" + + +def test_reworded_marker_without_value_swap_does_not_invalidate(): + """A change marker on a candidate that shares only loose subject nouns + with the neighbour is not correction evidence. Common words like "now" + leak into every sentence, and surface noun overlap ("production API") + does not imply predicate agreement ("uses Redis caching" vs "uses + three replicas"). The marker leg now requires a value_swap on the + same shared subject — predicate and value together, not just + predicate and marker. + """ + neighbor = _rec("The production API uses Redis caching for user sessions.", + id="mem_cache") + res = resolve("The production API now uses three replicas for high availability.", + [(0.45, neighbor)]) + assert res.op != ResolutionOp.INVALIDATE + assert res.target_id != "mem_cache" + + +def test_date_swap_correction_invalidates_when_attribute_is_shared(): + neighbor = _rec("Until January the rate limit was 100 requests per minute.", + id="mem_old_rate") + res = resolve("As of February the rate limit is 500 requests per minute.", + [(0.55, neighbor)]) + assert res.op == ResolutionOp.INVALIDATE + assert res.target_id == "mem_old_rate" + + +def test_distinct_environment_values_never_invalidate_each_other(): + # Same attribute, different environment qualifier: two coexisting facts. + neighbor = _rec("Redis cache TTL is 300 seconds in staging.", id="mem_ttl_staging") + res = resolve("Redis cache TTL is 3600 seconds in production.", [(0.7, neighbor)]) + assert res.op != ResolutionOp.INVALIDATE + + +def test_named_identifier_swap_is_not_proven_a_correction_without_marker(): + # ProviderA -> ProviderB beside 4 -> 8 workers could be parallel infrastructure; + # the hashing embedder cannot prove the same predicate, so both stay live. + neighbor = _rec("CI runs on ProviderA with 4 workers.", id="mem_ci_workers") + res = resolve("CI runs on ProviderB with 8 workers.", [(0.65, neighbor)]) + assert res.op != ResolutionOp.INVALIDATE + + +def test_clean_noun_swap_vetoes_strong_joint_invalidation(): + # Pre-existing false-invalidation class: strong overlap+cosine, but the diff + # replaces one plain noun with another and no value changes. + neighbor = _rec("The docs cover the REST interface.", id="mem_docs_rest") + res = resolve("The docs cover the GraphQL interface.", [(0.9, neighbor)]) + assert res.op == ResolutionOp.RELATE + + +def test_distinct_attribute_with_numbers_stays_live(): + # "refreshes every 5 minutes" vs "holds about 2 million documents": numbers + # change but they belong to different attributes (no shared value anchor). + neighbor = _rec("The search index refreshes every 5 minutes.", id="mem_index_refresh") + res = resolve("The search index holds about 2 million documents.", [(0.6, neighbor)]) + assert res.op != ResolutionOp.INVALIDATE + + +def test_engine_reworded_correction_closes_the_stale_fact_end_to_end(): + from engraphis.core.engine import MemoryEngine + eng = MemoryEngine.create(":memory:", auto_evolve=False) + try: + wid = eng.store.get_or_create_workspace("w") + rid = eng.store.get_or_create_repo(wid, "r") + old = eng.remember_with_resolution( + "The request timeout is 30 seconds.", workspace_id=wid, repo_id=rid) + new = eng.remember_with_resolution( + "We raised the request timeout to 90 seconds last sprint.", + workspace_id=wid, repo_id=rid) + assert new["op"] == "invalidate" + assert new["superseded"] == [old["id"]] + assert eng.store.get_memory(old["id"]).valid_to is not None + assert eng.store.get_memory(new["id"]).valid_to is None + finally: + eng.store.close() + + +def test_engine_distinct_facts_about_one_topic_both_stay_live(): + from engraphis.core.engine import MemoryEngine + eng = MemoryEngine.create(":memory:", auto_evolve=False) + try: + wid = eng.store.get_or_create_workspace("w") + rid = eng.store.get_or_create_repo(wid, "r") + budget = eng.remember_with_resolution( + "The data migration budget is 50 thousand dollars.", + workspace_id=wid, repo_id=rid) + deadline = eng.remember_with_resolution( + "The data migration deadline is March 15.", + workspace_id=wid, repo_id=rid) + assert deadline["op"] in ("add", "relate") + assert eng.store.get_memory(budget["id"]).valid_to is None + assert eng.store.get_memory(deadline["id"]).valid_to is None + finally: + eng.store.close() + + +# ── R1 review: additional safety-class tests for reworded-correction paths ────── + + +def test_distinct_environment_values_never_invalidate_via_strong_branch_long_form(): + # The short-form env-conflict test (Redis TTL 300 -> 3600 in staging/production) + # happened to split into two diff spans; this long-form variant collapses to a + # single replace span. R1 review found the strong branch's swap_veto did not + # honour env_conflict, so the strong path would incorrectly supersede. + neighbor = _rec( + "The primary database connection pool in the staging environment holds " + "5 connections per application instance under nominal load.", + id="mem_staging_pool", + ) + res = resolve( + "The primary database connection pool in the production environment holds " + "8 connections per application instance under nominal load.", + [(0.7, neighbor)], + ) + assert res.op != ResolutionOp.INVALIDATE + + +def test_marker_with_value_swap_invalidates(): + # The marker leg now requires a value_swap on the same shared subject + # — marker + value is a real correction ("now runs 6pm" rewrites + # "runs 5pm"). A bare marker without a value change stays ADD. + neighbor = _rec("Deploy schedule runs at 5pm on Fridays.", id="mem_deploy_v") + res = resolve("Deploy schedule now runs at 6pm on Fridays.", + [(0.6, neighbor)]) + assert res.op == ResolutionOp.INVALIDATE + assert res.target_id == "mem_deploy_v" + + +def test_marker_alone_without_value_swap_does_not_invalidate(): + # "We migrated the docs to cover the GraphQL interface" against + # "The docs cover the REST interface" has a marker and a heavy noun + # swap but no value_swap — different facts about a similar topic, + # not the same fact restated. Stays ADD. + neighbor = _rec("The docs cover the REST interface.", id="mem_docs_rest") + res = resolve("We migrated the docs to cover the GraphQL interface.", + [(0.9, neighbor)]) + assert res.op != ResolutionOp.INVALIDATE + + +def test_closed_predecessor_supersedes_under_strong_evidence_regardless_of_prose(): + # A closed (valid_to set) predecessor is historical chain membership: even + # the strong path's swap_vetoes should not protect a closed record. This + # is the backfill-and-supersede contract: "the chain wants this rewrite". + closed = _memory_obj( + id="mem_closed_old", + content="Deploy schedule runs at 5pm on Fridays.", + valid_to=1_000.0, + ) + res = resolve("Deploy schedule now runs at 6pm on Fridays.", + [(0.9, closed)]) + assert res.op == ResolutionOp.INVALIDATE + assert res.target_id == "mem_closed_old" + + +def _memory_obj(id: str, content: str, *, valid_to=None): + """Minimal in-memory MemoryRecord with the fields _rec doesn't expose.""" + from engraphis.core.interfaces import MemoryRecord + return MemoryRecord( + id=id, workspace_id="w", repo_id=None, session_id=None, + title="", content=content, mtype="semantic", scope="workspace", + importance=0.0, confidence=1.0, valid_from=0.0, valid_to=valid_to, + ingested_at=0.0, expired_at=None, + subject_key="", claim_kind="", keywords=(), metadata={}, + provenance={"source": "agent", "trusted": True, "review_state": "approved"}, + ) diff --git a/tests/test_session_start_hook.py b/tests/test_session_start_hook.py new file mode 100644 index 00000000..baf1a296 --- /dev/null +++ b/tests/test_session_start_hook.py @@ -0,0 +1,116 @@ +# -*- coding: utf-8 -*- +"""Unit tests for integrations.commandcode.session_start_hook. + +These tests exercise the pure-function surface (resolve_workspace, +build_additional_context) plus the JSON-RPC error paths. They do NOT call the +real MCP server; live integration is covered by the rebench end-to-end proof. +""" +import importlib.util +import io +import json +import os +import sys +import unittest +from unittest import mock + +ROOT = os.path.dirname(os.path.abspath(__file__)) +HOOK = os.path.normpath(os.path.join( + ROOT, "..", "integrations", "commandcode", "session_start_hook.py")) + + +def _load(): + spec = importlib.util.spec_from_file_location("session_start_hook", HOOK) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class WorkspaceResolution(unittest.TestCase): + def setUp(self): + self.hook = _load() + + def test_default_falls_back_to_repo_basename(self): + self.assertEqual( + self.hook.resolve_workspace("C:/work/engraphis", {}), + "engraphis", + ) + + def test_override_wins_over_basename(self): + self.assertEqual( + self.hook.resolve_workspace( + "C:/work/engraphis", {"ENGRAPHIS_HOOK_WORKSPACE": "ops-prod"}), + "ops-prod", + ) + + def test_blank_override_falls_back(self): + self.assertEqual( + self.hook.resolve_workspace( + "C:/work/engraphis", {"ENGRAPHIS_HOOK_WORKSPACE": " "}), + "engraphis", + ) + + +class ContextBuild(unittest.TestCase): + def setUp(self): + self.hook = _load() + + def test_header_carries_workspace_name(self): + out = self.hook.build_additional_context("hello", "ops-prod") + self.assertIn("workspace ops-prod", out) + self.assertIn("hello", out) + self.assertIn("mcp__engraphis__", out) + + def test_truncates_at_budget(self): + original = self.hook.MAX_CONTEXT_CHARS + self.hook.MAX_CONTEXT_CHARS = 50 + try: + out = self.hook.build_additional_context("x" * 10_000, "ws") + finally: + self.hook.MAX_CONTEXT_CHARS = original + # Header + footer + truncated body, never exceeds the budget. + self.assertLessEqual(len(out), 50) + + +class EndToEndBehavior(unittest.TestCase): + def setUp(self): + self.hook = _load() + + def test_fails_open_on_unreachable_server(self): + with mock.patch.object(self.hook, "MCP_URL", "http://127.0.0.1:9/mcp"): + with mock.patch.object(sys, "stdin", io.StringIO(json.dumps({ + "hook_event_name": "SessionStart", + "cwd": "C:/work/engraphis", + }))): + buf = io.StringIO() + with mock.patch.object(sys, "stdout", buf): + self.assertEqual(self.hook.main(), 0) + self.assertEqual(buf.getvalue(), "") + + def test_wrong_event_prints_nothing(self): + with mock.patch.object(sys, "stdin", io.StringIO(json.dumps({ + "hook_event_name": "PreToolUse", + "cwd": "C:/work/engraphis", + }))): + buf = io.StringIO() + with mock.patch.object(sys, "stdout", buf): + self.assertEqual(self.hook.main(), 0) + self.assertEqual(buf.getvalue(), "") + + def test_workspace_override_used_in_call(self): + with mock.patch.object(self.hook, "MCP_URL", "http://127.0.0.1:9/mcp"): + with mock.patch.object(self.hook, "session_context", + return_value="") as fake: + with mock.patch.dict(os.environ, + {"ENGRAPHIS_HOOK_WORKSPACE": "ops"}): + with mock.patch.object(sys, "stdin", io.StringIO(json.dumps({ + "hook_event_name": "SessionStart", + "cwd": "C:/work/engraphis", + }))): + buf = io.StringIO() + with mock.patch.object(sys, "stdout", buf): + self.hook.main() + self.assertEqual(fake.call_args.args[1], "ops") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_smart_mcp_gateway.py b/tests/test_smart_mcp_gateway.py index 836988ac..267c8334 100644 --- a/tests/test_smart_mcp_gateway.py +++ b/tests/test_smart_mcp_gateway.py @@ -92,8 +92,12 @@ def test_normal_mcp_exposes_only_the_smart_gateway_tools(monkeypatch): tools = _tools(server, "mcp") assert set(tools) == SMART_TOOL_NAMES assert len(tools) == 9 - assert len(server.mcp.instructions) <= 512 assert "scope" not in tools["engraphis_remember"].inputSchema.get("properties", {}) + # The documented safe-supersession mechanism must stay reachable through the + # served gateway surface: subject_key/claim_kind forward to the classic tool. + props = tools["engraphis_remember"].inputSchema.get("properties", {}) + assert "subject_key" in props + assert "claim_kind" in props def test_classic_mcp_retains_the_34_named_tool_compatibility_surface(monkeypatch): @@ -485,6 +489,42 @@ def test_smart_session_start_and_end_preserve_handoff_contract(monkeypatch): assert ended["status"] == "summarized" +def test_smart_session_action_normalizes_full_tool_name(monkeypatch): + """The Command Code harness passes action='start_session'/'end_session' + (the full tool name) when translating the AGENTS.md engraphis_start_session + / engraphis_end_session shorthand. Both the direct-call path and the MCP + protocol validation path must accept and normalize these.""" + server = _memory_server(monkeypatch) + + # Direct call: function-body normalization handles it. + started = _payload(server.engraphis_session( + action="start_session", workspace="acme", repo="api", agent="test-agent", + )) + assert started["status"] == "active" + assert started["workspace"] == "acme" + + ended = _payload(server.engraphis_session( + action="end_session", session_id=started["session_id"], summary="Done.", + )) + assert ended["status"] == "summarized" + assert ended["session_id"] == started["session_id"] + + +def test_smart_session_arg_model_accepts_full_tool_name(monkeypatch): + """BeforeValidator normalizes start_session/end_session before the pattern + constraint, so the MCP protocol validation path doesn't reject them.""" + from mcp.server.fastmcp.utilities.func_metadata import func_metadata + + server = _memory_server(monkeypatch) + meta = func_metadata(server.engraphis_session) + # The raw harness value that triggers the validation error in production. + assert meta.arg_model.model_validate({"action": "start_session"}).action == "start" + assert meta.arg_model.model_validate({"action": "end_session"}).action == "end" + # Genuine invalid actions are still rejected by the pattern. + with pytest.raises(ValueError): + meta.arg_model.model_validate({"action": "bogus"}) + + def test_gateway_not_found_failure_returns_iserror_envelope(monkeypatch): """A missing memory surfaced through the gateway is E_NOT_FOUND.""" server = _memory_server(monkeypatch)