Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions Gradata/src/gradata/hooks/implicit_feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,16 @@ def main(data: dict) -> dict | None:
{"mode": "tacit", "message_preview": message[:200]},
)

# Feedback signals are logged via emit_hook_event above; no inline
# context injection needed — the learning pipeline reads events.jsonl.
# Suppressing the [fb:neg,rem] result saves ~1.75 tok/turn avg.
if signals:
_SIG_ABBREV = {
"negation": "neg",
"reminder": "rem",
"challenge": "chal",
"approval": "approv",
"gap": "gap",
}
sig_str = ",".join(_SIG_ABBREV.get(str(s["type"]), str(s["type"])) for s in signals)
return {"result": f"[fb:{sig_str}]"}
Comment on lines +205 to +214
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Resolve conflicting approval + negative signals before emitting/returning feedback.

Line 205 currently returns all detected signal types, but the current flow can classify a single message as both negative and approval (e.g., challenge phrasing containing “that’s correct”). That can emit OUTPUT_ACCEPTED (Line 188) and also return negative feedback in the same turn, which is contradictory.

Suggested fix
@@
-        has_negative = bool(signal_types & _NEGATIVE_SIGNAL_TYPES)
-        has_approval = "approval" in signal_types
+        has_negative = bool(signal_types & _NEGATIVE_SIGNAL_TYPES)
+        has_approval = "approval" in signal_types
+
+        # Negative feedback must take precedence over approval to avoid
+        # contradictory acceptance + correction in the same message.
+        if has_negative and has_approval:
+            signals = [s for s in signals if s["type"] != "approval"]
+            signal_types.discard("approval")
+            has_approval = False
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/src/gradata/hooks/implicit_feedback.py` around lines 205 - 214, The
code can emit both an approval and negative feedback at once; update the signals
handling in implicit_feedback.py (the block that builds _SIG_ABBREV and sig_str
and returns {"result": ...}) to resolve conflicts by preferring approval: if any
signal with type "approval" exists, filter out negative signal types (e.g.,
"negation", "challenge", "gap") before constructing sig_str so you won't return
negative feedback alongside OUTPUT_ACCEPTED; keep the existing _SIG_ABBREV
mapping and sig_str construction but operate on a cleaned signals list (or set)
so the return only includes the resolved signal types.

return None
except Exception as exc:
_log.debug("implicit_feedback hook error: %s", exc)
Expand Down
15 changes: 5 additions & 10 deletions Gradata/src/gradata/hooks/inject_brain_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,10 @@ def _read_brain_prompt(brain_dir: Path) -> str | None:
result.append(line)
i += 1
text = "\n".join(result)
# Strip lower-priority sections (Active guidance, Current disposition).
# Non-negotiables are the hardest constraints and are sufficient for session
# context; the guidance/disposition sections are ~140 tokens of softer context
# that the JIT hook covers per-prompt when relevant. Saves ~140 tok/session.
# Opt back in with GRADATA_WISDOM_FULL=1 for ablation.
if os.environ.get("GRADATA_WISDOM_FULL", "0") != "1":
# Active guidance + Current disposition sections kept by default — they
# carry softer behavioral context the model needs at session start. Set
# GRADATA_WISDOM_FULL=0 to strip them (ablation only).
if os.environ.get("GRADATA_WISDOM_FULL", "1") != "1":
for marker in ("Active guidance", "Current disposition"):
idx = text.find(marker)
if idx != -1:
Expand All @@ -184,10 +182,7 @@ def _read_brain_prompt(brain_dir: Path) -> str | None:
count=1,
)
# Limit to first GRADATA_WISDOM_MAX_RULES non-negotiable rules.
# Reduced 11→9→6→3: keep only the top-3 "Never" attribution/data/booking rules
# which address the highest-stakes errors. Mid-tier rules fire via JIT when
# contextually relevant and are retrievable via brain.search(). Saves ~59 tok.
wisdom_max_rules = int(os.environ.get("GRADATA_WISDOM_MAX_RULES", "3"))
wisdom_max_rules = int(os.environ.get("GRADATA_WISDOM_MAX_RULES", "9"))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Harden GRADATA_WISDOM_MAX_RULES parsing to prevent SessionStart breakage.

Line 185 uses raw int(...) on environment input. A malformed value (e.g. "abc") raises ValueError and can abort injection instead of degrading safely.

Proposed defensive parse + clamp
-    wisdom_max_rules = int(os.environ.get("GRADATA_WISDOM_MAX_RULES", "9"))
+    raw_max_rules = os.environ.get("GRADATA_WISDOM_MAX_RULES", "9").strip()
+    try:
+        wisdom_max_rules = max(0, int(raw_max_rules))
+    except ValueError:
+        wisdom_max_rules = 9

Based on learnings: _env_int default clamping to minimum.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/src/gradata/hooks/inject_brain_rules.py` at line 185, Replace the raw
int(...) parsing of GRADATA_WISDOM_MAX_RULES used to set wisdom_max_rules with a
defensive parse: catch ValueError/TypeError, fall back to the default (9), and
clamp the resulting value to a safe minimum (e.g., 0 or 1) and optionally an
upper bound; you can reuse or implement a small helper like _env_int to perform
parse-with-default-and-clamp. Update the code that references wisdom_max_rules
(in inject_brain_rules.py / SessionStart injection) to use this safe value so
malformed env input won’t raise and abort injection.

if wisdom_max_rules > 0:
rule_lines = [ln for ln in text.split("\n") if ln.startswith("- ")]
if len(rule_lines) > wisdom_max_rules:
Expand Down
19 changes: 8 additions & 11 deletions Gradata/src/gradata/hooks/jit_inject.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,8 @@
}

# Defaults. All tunable by env var so operators can sweep without a code change.
# Reduced 5→3→2→1: inject only the single best-matching rule per turn.
# The top-1 BM25 hit carries the dominant signal; marginal rules add noise.
# Saves ~16 tok/turn over k=2 (expected ~160 weighted_tokens).
DEFAULT_MAX_RULES = 1
# Raised 0.60→0.90: rules below 0.90 are softer guidance (PATTERN tier) already
# covered by the Active guidance section in the wisdom block or not high-signal
# enough for per-turn injection. Rules ≥0.90 (RULE tier) in brain_prompt.md are
# already in the session wisdom block, so the wisdom-dedup step will filter them.
# Net effect: JIT fires only for novel RULE-tier rules outside the wisdom block.
DEFAULT_MIN_CONFIDENCE = 0.90
DEFAULT_MAX_RULES = 5
DEFAULT_MIN_CONFIDENCE = 0.60
DEFAULT_MIN_SIMILARITY = 0.05
MIN_DRAFT_LEN = 10

Expand Down Expand Up @@ -371,6 +363,10 @@ def _already_in_wisdom(desc: str) -> bool:
return False

# Dedup by normalized description AND by overlap with session wisdom block.
# Emit as `[P83] description` — state abbreviation (P=PATTERN, I=INSTINCT,
# R=RULE) + confidence percent. Keeps state+confidence signal for the model
# while remaining compact (~3 tok/rule overhead).
_STATE_ABBREV = {"PATTERN": "P", "INSTINCT": "I", "RULE": "R"}
seen_descs: set[str] = set()
lines = []
for r, _sim in ranked:
Expand All @@ -380,7 +376,8 @@ def _already_in_wisdom(desc: str) -> bool:
seen_descs.add(norm_desc)
if _already_in_wisdom(r.description):
continue
lines.append(r.description)
prefix = f"[{_STATE_ABBREV.get(r.state.name, r.state.name)}{round(r.confidence * 100):02d}]"
lines.append(f"{prefix} {r.description}")
if not lines:
return None
rules_block = "\n".join(lines)
Expand Down
8 changes: 4 additions & 4 deletions Gradata/tests/test_hooks_intelligence.py
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ def test_implicit_feedback_detects_negation(tmp_path, monkeypatch):
monkeypatch.setenv("GRADATA_BRAIN_DIR", str(tmp_path))
with patch("gradata.hooks.implicit_feedback.emit_hook_event") as mock_emit:
result = feedback_main({"message": "No, that's wrong. Do it differently."})
assert result is None
assert result == {"result": "[fb:neg]"}
event_types = [call.args[0] for call in mock_emit.call_args_list]
assert "IMPLICIT_FEEDBACK" in event_types
signals = mock_emit.call_args_list[0].args[2]["signals"]
Expand All @@ -454,7 +454,7 @@ def test_implicit_feedback_detects_reminder(tmp_path, monkeypatch):
monkeypatch.setenv("GRADATA_BRAIN_DIR", str(tmp_path))
with patch("gradata.hooks.implicit_feedback.emit_hook_event") as mock_emit:
result = feedback_main({"message": "I told you to always plan first before building."})
assert result is None
assert result == {"result": "[fb:rem]"}
event_types = [call.args[0] for call in mock_emit.call_args_list]
assert "IMPLICIT_FEEDBACK" in event_types
signals = mock_emit.call_args_list[0].args[2]["signals"]
Expand All @@ -465,7 +465,7 @@ def test_implicit_feedback_detects_challenge(tmp_path, monkeypatch):
monkeypatch.setenv("GRADATA_BRAIN_DIR", str(tmp_path))
with patch("gradata.hooks.implicit_feedback.emit_hook_event") as mock_emit:
result = feedback_main({"message": "Are you sure that's correct? It doesn't look right."})
assert result is None
assert result is not None and "chal" in result["result"]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Tighten these assertions to prevent false-positive passes.

Line 468 and Line 486 currently allow broad outputs, so conflicting or extra tags can slip through unnoticed. Prefer exact expected payloads for deterministic regression checks.

Suggested test tightening
-    assert result is not None and "chal" in result["result"]
+    assert result == {"result": "[fb:chal]"}
@@
-    assert result is not None and result["result"].startswith("[fb:")
+    assert result == {"result": "[fb:rem,chal]"}

Also applies to: 486-486

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Gradata/tests/test_hooks_intelligence.py` at line 468, The current assertion
"assert result is not None and 'chal' in result['result']" is too loose and can
yield false positives; update the test in test_hooks_intelligence (the assertion
referencing the variable result and its "result" key, and the similar assertion
around line 486) to assert exact expected outputs — either compare result to the
full expected dictionary payload or assert result["result"] equals the exact
expected string and also validate any tag lists/sets (e.g., compare sets) to
ensure no extra or missing tags are present; make the assertions deterministic
and strict instead of using a simple substring membership.

event_types = [call.args[0] for call in mock_emit.call_args_list]
assert "IMPLICIT_FEEDBACK" in event_types
signals = mock_emit.call_args_list[0].args[2]["signals"]
Expand All @@ -483,7 +483,7 @@ def test_implicit_feedback_emits_event(tmp_path):
patch("gradata.hooks.implicit_feedback.emit_hook_event") as mock_emit,
):
result = feedback_main({"message": "I told you not to do that, are you sure?"})
assert result is None
assert result is not None and result["result"].startswith("[fb:")
event_types = [call.args[0] for call in mock_emit.call_args_list]
assert "IMPLICIT_FEEDBACK" in event_types

Expand Down
Loading