fix(hook): cap the soft read nudge per session and suppress it after a fresh query - #3498
fix(hook): cap the soft read nudge per session and suppress it after a fresh query#3498Ashfaqbs wants to merge 1 commit into
Conversation
…a fresh query The PreToolUse read nudge (_READ_NUDGE) fired unconditionally on every qualifying Read, with no dedup, no session cap, and no suppression while the agent had recently run graphify query/explain/path. Only the opt-in strict deny was capped, via _mark_session_denied. Measured across 87 transcripts in one repo: 8,348 nudge injections producing 339 graphify calls (~4% response rate), at a standing cost of ~651k tokens - about 3.7x every graphify query in those sessions combined. Fix: - Suppress the nudge entirely while _query_stamp_fresh() is true (an agent that just oriented does not need reminding). - Otherwise cap it to once per session per GRAPHIFY_HOOK_NUDGE_TTL seconds (default 300), via a new _nudge_recently_shown helper that mirrors the existing _mark_session_denied marker pattern. Fails open: any error still shows the nudge. Fixes Graphify-Labs#3435.
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Suppresses the soft PreToolUse read nudge when a recent graphify query/explain/path stamp is fresh, and caps it via _nudge_recently_shown to once per session per GRAPHIFY_HOOK_NUDGE_TTL seconds (default 300), where before it re-fired on every qualifying read. The cap is keyed on a sanitized session id written to a hook_sessions marker file with best-effort GC of markers older than 24h; it fails open (no session id or any error means the nudge still shows) so orientation reminders are never wrongly dropped.
No blocking issues surfaced. 5 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 486 functions depend on the 288 functions this change touches.
Health — this change adds coupling hotspots:
- new:
dispatch_command()— 2 callers, 124 callees - new:
_stale_graph_sources()— 7 callers, 6 callees - new:
_run_hook_guard()— 4 callers, 9 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 486 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 428 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
268 of 268 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/test_affected_cli.py— impact, full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— impact, full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_build.py— full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— full-run-safetytests/test_chunking.py— full-run-safetytests/test_cjs_module_extension.py— full-run-safetytests/test_claude_cli_backend.py— full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— full-run-safetytests/test_codebuddy.py— impact, full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_nested_and_cli.py— full-run-safetytests/test_cpp_objc_cross_file_calls.py— full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— full-run-safetytests/test_cross_language_call_resolution.py— full-run-safetytests/test_cross_repo_member_calls.py— full-run-safetytests/test_cross_repo_shared_types.py— full-run-safetytests/test_csharp_call_site_generic_args.py— full-run-safetytests/test_csharp_enum_members.py— full-run-safetytests/test_csharp_field_generic_args.py— full-run-safety- … and 218 more
non-code file(s) changed (
CHANGELOG.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
CHANGELOG.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
No difference found (not proven): No behavior difference found in \_run\_hook\_guard (not a proof).
The verifier ran both versions of \_run\_hook\_guard on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 4 more finding(s) on lines outside this diff (see the check run).
1 participant
Fixes #3435.
Problem
The soft PreToolUse read nudge (
_READ_NUDGE) fired unconditionally on every qualifying Read, with no dedup, no session cap, and no suppression while the agent had recently rungraphify query/explain/path. Only the opt-in strict deny was capped, via_mark_session_denied.Measured across 87 Claude Code transcripts in one repo: 8,348 nudge injections producing 339 graphify calls (~4% response rate), at a standing cost of ~651k tokens — about 3.7x every graphify query in those sessions combined.
Fix
_query_stamp_fresh()is true — an agent that just oriented (query/explain/path) does not need reminding on its very next read.GRAPHIFY_HOOK_NUDGE_TTLseconds (default 300), via a new_nudge_recently_shownhelper that mirrors the existing_mark_session_deniedmarker-file pattern (best-effort GC of markers older than 24h, fails open on any error so the nudge is never wrongly suppressed).Both suggested fixes from the issue (#1 and #2, the two highest value-to-effort items) are implemented; dedup-per-target and payload shortening (#3/#4) are left out to keep this PR narrowly scoped to the two changes that account for "well over 90%" of the measured cost per the issue's own estimate.
Tests
test_fresh_query_stamp_suppresses_deny(renamed..._and_nudge) — a fresh query stamp now suppresses both the strict deny and the soft nudge, since the old assertion documented the exact bug being fixed.test_fresh_query_stamp_suppresses_soft_nudge_too— same suppression applies outside strict mode.test_soft_nudge_capped_per_session— a second read in the same session within the TTL produces no output.test_soft_nudge_cap_is_per_session— a different session id still gets the nudge.test_soft_nudge_cap_expires_after_ttl— an expired marker (backdated viaos.utime) lets the nudge fire again.test_soft_nudge_never_capped_without_session_id— fail-open: no session id means no cap is possible, so the nudge keeps firing rather than risk silently swallowing it.uv run pytest tests/test_hook_strict.py tests/test_hook_guard.py tests/test_hook_guard_token_match.py tests/test_hook_out_of_project_paths.py tests/test_install_strings.py -q— 162 passed.Full suite (
uv run pytest -q): 5410 passed, 30 failed, 99 skipped. All 30 failures are pre-existing and unrelated to this change (Windows-platform-specific: FIFO/unix-socket tests that don't apply on Windows, backslash path-separator assertions, shebang parsing) — none touchgraphify/cli.py's hook-guard code ortests/test_hook_strict.py.Caveats
GRAPHIFY_HOOK_NUDGE_TTLdefault of 300s (5 minutes) is a judgment call, not something I could tune against real usage data beyond the one repo's transcripts cited in the issue. Happy to adjust if a maintainer has a better sense of the right default._READ_NUDGE) reported in PreToolUse soft nudge has no session cap — measured 8,348 injections (~651k tokens) in a single project #3435. The search nudge (_SEARCH_NUDGE) has the same unconditional-fire shape but wasn't part of the measured cost in the issue, so I left it untouched rather than expanding scope.