-
Notifications
You must be signed in to change notification settings - Fork 113
fix: clear empty husks for pending prompts; if no graph, debug recall log, not error #379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
siillee
merged 2 commits into
main
from
feature/sdk-469-pending-dir-accumulates-empty-husk-files-graph-scope-404
Aug 31, 2026
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
60 changes: 60 additions & 0 deletions
60
integrations/tests/tests/e2e/test_recall_graph_not_built.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| """A fresh dataset's graph scope answers 404 — that is not a recall error (SDK-469, part 2). | ||
|
|
||
| Until the first cognify lands, the server has no graph for the dataset and | ||
| answers the graph scope with 404. On a fresh install that is every prompt of the | ||
| first session, and it used to log ``recall_error {verdict: unknown}`` each time — | ||
| pure noise that also fed the health accounting. The hook now records it as | ||
| ``recall_graph_not_built`` and leaves ``recall_error`` for real failures. | ||
|
|
||
| The mock's forced 404 applies to every scope, which is what makes the assertion | ||
| sharp: the graph scope must be the only one *not* reported as an error. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json | ||
|
|
||
| from utils.suites import state_dir | ||
|
|
||
|
|
||
| def _events(suite, home): | ||
| log = state_dir(suite, home) / "hook.log" | ||
| if not log.exists(): | ||
| return [] | ||
| out = [] | ||
| for line in log.read_text(encoding="utf-8").splitlines(): | ||
| try: | ||
| entry = json.loads(line) | ||
| except ValueError: | ||
| continue | ||
| out.append((entry.get("event"), entry.get("detail") or {})) | ||
| return out | ||
|
|
||
|
|
||
| def test_graph_404_is_not_a_recall_error( | ||
| suite, run_hook, mock_server, payloads, temp_home, assert_clean_real_home | ||
| ): | ||
| mock_server.force_response("POST", "/api/v1/recall", 404, {"detail": "DatasetNotFoundError"}) | ||
| result = run_hook( | ||
| suite, | ||
| "session-context-lookup.py", | ||
| stdin=payloads.user_prompt(prompt="what did we decide about the retry policy?"), | ||
| service_url=mock_server.url, | ||
| # The graph scope runs last, and the hook stops dispatching scopes once its | ||
| # per-prompt budget (default 4s) is spent. On the Windows runner every | ||
| # request to the mock takes ~2s, so with the defaults only two scopes ran | ||
| # and the graph scope — the one this test is about — was never attempted. | ||
| # The budget is a production latency guard, not the behaviour under test. | ||
| env={"COGNEE_RECALL_TIMEOUT": "30", "COGNEE_RECALL_BUDGET": "120"}, | ||
| ) | ||
| assert result.returncode == 0, result.stderr | ||
|
|
||
| events = _events(suite, temp_home) | ||
| assert not [d for e, d in events if e == "recall_budget_exceeded"], ( | ||
| "the budget must not cut the scope loop short in this test" | ||
| ) | ||
| not_built = [d for e, d in events if e == "recall_graph_not_built"] | ||
| errors = [d for e, d in events if e == "recall_error"] | ||
| assert not_built and not_built[0]["scope"] == ["graph"] | ||
| assert errors, "the other scopes still 404 in this forced setup and must still be reported" | ||
| assert all(d["scope"] != ["graph"] for d in errors), errors |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| """The pending-prompt buffer leaves nothing behind (SDK-469, part 1). | ||
|
|
||
| ``remember_pending_prompt`` parks a prompt until the Stop hook pops it. The pop | ||
| used to write the emptied dict back as ``{}``, leaving one 2-byte husk per | ||
| session forever (80 of 88 files in one pending/ dir). Now the last pop removes | ||
| the file, a pop against nothing creates nothing, and the SessionStart sweep | ||
| clears husks older versions left — immediately, since an empty buffer has | ||
| nothing in flight. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import pytest | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def pc(suite, isolated_modules, monkeypatch): | ||
| module = isolated_modules(suite, "_plugin_common") | ||
| monkeypatch.setattr(module, "hook_log", lambda *a, **k: None) | ||
| monkeypatch.setenv("COGNEE_SESSION_KEY", "host-abc") | ||
| return module | ||
|
|
||
|
|
||
| def test_last_pop_removes_the_file(pc): | ||
| pc.remember_pending_prompt("s1", "what did we decide?", turn_id="t1") | ||
| path = pc._pending_file("s1") | ||
| assert path.exists() | ||
| popped = pc.pop_pending_prompt("s1", turn_id="t1") | ||
| assert popped["prompt"] == "what did we decide?" | ||
| assert not path.exists(), "an emptied buffer must not stay behind as {}" | ||
|
|
||
|
|
||
| def test_pop_keeps_the_file_while_other_turns_are_pending(pc): | ||
| pc.remember_pending_prompt("s1", "first", turn_id="t1") | ||
| pc.remember_pending_prompt("s1", "second", turn_id="t2") | ||
| path = pc._pending_file("s1") | ||
| assert pc.pop_pending_prompt("s1", turn_id="t1")["prompt"] == "first" | ||
| assert path.exists() | ||
| assert pc.pop_pending_prompt("s1", turn_id="t2")["prompt"] == "second" | ||
| assert not path.exists() | ||
|
|
||
|
|
||
| def test_pop_against_nothing_creates_nothing(pc): | ||
| path = pc._pending_file("s1") | ||
| assert pc.pop_pending_prompt("s1", turn_id="t9") == {"prompt": "", "context": ""} | ||
| assert not path.exists() | ||
|
|
||
|
|
||
| def test_sweep_removes_husks_regardless_of_age(pc): | ||
| pc._PENDING_DIR.mkdir(parents=True, exist_ok=True) | ||
| husk = pc._PENDING_DIR / "old-session.json" | ||
| husk.write_text("{}", encoding="utf-8") | ||
| live = pc._PENDING_DIR / "live.json" | ||
| live.write_text('{"host:t1": {"prompt": "x"}}', encoding="utf-8") | ||
| counts = pc.sweep_stale_state() | ||
| assert counts.get("pending_husks") == 1 | ||
| assert not husk.exists() and live.exists() |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.