feat(memory): procedural-content gate + bypass flag + tests + skill docs - #30
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 40 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between cd430d8fec61ed40c12fb69a53fb80444db356a1 and b58f85c. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds a procedural-content gate to the memory tool that prevents procedure-like, recipe-like, and skill-duplication text from being stored in durable memory. The gate identifies patterns via regex and keyword heuristics, with a narrow bypass flag for legitimate environment facts. The feature is exposed via updated method signatures, OpenAI schema, comprehensive test coverage, and user-facing documentation. ChangesProcedural-content Gate for Memory Durability
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-import |
1 |
First entries
tests/tools/test_memory_procedural_gate.py:8: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
✅ Fixed issues: none
Unchanged: 5040 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tools/memory_tool.py (1)
830-830: ⚡ Quick winHarden
bypass_procedural_checkagainst non-boolean strings
model_tools.handle_function_call()runscoerce_tool_args()usingMEMORY_SCHEMA, which converts string"true"/"false"to real booleans before dispatch—sobool("false")won’t bypass in the normal tool-call path. However, if the value arrives as another non-empty string (e.g."0"/"no") it won’t be coerced andbool(<string>)will still evaluateTrue, bypassing the procedural gate attools/memory_tool.py:830.Suggested fix
- bypass_procedural_check=bool(args.get("bypass_procedural_check", False)), + bypass_procedural_check=( + args.get("bypass_procedural_check", False) + if isinstance(args.get("bypass_procedural_check", False), bool) + else False + ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/memory_tool.py` at line 830, The bypass_procedural_check value is being set with bool(args.get(...)) which treats any non-empty string (e.g. "0", "no") as True and can unintentionally bypass the procedural gate; update the assignment for bypass_procedural_check in tools/memory_tool.py to normalize string inputs (check type and if a string, lower() and compare against a whitelist like {"true","1","yes","y","t"} for True and {"false","0","no","n","f"} for False) or parse with a canonical string-to-bool helper used by model_tools.handle_function_call/coerce_tool_args/MEMORY_SCHEMA, so that string forms are coercively converted to real booleans before being used.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/tools/test_memory_procedural_gate.py`:
- Around line 140-148: The test currently has no assertions so it always passes;
update test_bypass_env_fact_with_path to assert expected behavior: first assert
that _detect_procedural_content("AWS_PROFILE=mcp-hive points at
~/.aws/credentials") is False (or falsy) so the path-like env fact does not
trigger procedural detection, then exercise the MemoryStore bypass mechanism by
creating a MemoryStore instance, storing the same string using the store's
bypass option (e.g., MemoryStore.add_memory or MemoryStore.add with
bypass=True), and assert the memory was stored/retrievable (e.g., via
MemoryStore.get_memory or MemoryStore.retrieve) and that it is marked as
bypassed/not flagged as procedural; use the exact MemoryStore API available in
the test suite.
In `@tools/memory_tool.py`:
- Around line 134-135: The current regex in the memory rejection check (the if
that searches content and returns _PROCEDURAL_REJECTION_MSG) is too broad and
treats lone words like "update" or "delete" in normal prose as SQL; narrow the
heuristic by requiring SQL keywords to appear in SQL-like contexts (e.g.,
"UPDATE <identifier>\s+SET", "DELETE\s+FROM <identifier>", "INSERT\s+INTO
<identifier>", "CREATE\s+TABLE <identifier>", or a terminating semicolon) or by
requiring a keyword followed by an identifier or reserved token (FROM/SET/INTO);
update the re.search pattern accordingly to match those stricter forms against
the content variable and keep returning _PROCEDURAL_REJECTION_MSG only when the
stricter pattern matches.
---
Nitpick comments:
In `@tools/memory_tool.py`:
- Line 830: The bypass_procedural_check value is being set with
bool(args.get(...)) which treats any non-empty string (e.g. "0", "no") as True
and can unintentionally bypass the procedural gate; update the assignment for
bypass_procedural_check in tools/memory_tool.py to normalize string inputs
(check type and if a string, lower() and compare against a whitelist like
{"true","1","yes","y","t"} for True and {"false","0","no","n","f"} for False) or
parse with a canonical string-to-bool helper used by
model_tools.handle_function_call/coerce_tool_args/MEMORY_SCHEMA, so that string
forms are coercively converted to real booleans before being used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eeec5a08-6593-4b75-9d1a-872222d95109
📥 Commits
Reviewing files that changed from the base of the PR and between 056fb7e and 1908a83c56b70b996c3a187ef0a52f4c6e23ae62.
📒 Files selected for processing (3)
skills/autonomous-ai-agents/hermes-agent/SKILL.mdtests/tools/test_memory_procedural_gate.pytools/memory_tool.py
Comment on lines
+134
to
+135
| if re.search(r"\b(SELECT|INSERT|UPDATE|DELETE|CREATE\s+TABLE|ALTER\s+TABLE)\b", content, re.IGNORECASE): | ||
| return _PROCEDURAL_REJECTION_MSG |
There was a problem hiding this comment.
SQL heuristic is too broad and blocks normal prose.
Line 134 currently treats any standalone update/delete token as SQL. That will reject benign durable facts (e.g., “user prefers update reminders”), which undermines memory usefulness.
Suggested fix
- if re.search(r"\b(SELECT|INSERT|UPDATE|DELETE|CREATE\s+TABLE|ALTER\s+TABLE)\b", content, re.IGNORECASE):
+ if re.search(
+ r"(?is)(\bselect\b.+\bfrom\b|\binsert\s+into\b|\bupdate\b.+\bset\b|\bdelete\s+from\b|\bcreate\s+table\b|\balter\s+table\b)",
+ content,
+ ):
return _PROCEDURAL_REJECTION_MSG🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/memory_tool.py` around lines 134 - 135, The current regex in the memory
rejection check (the if that searches content and returns
_PROCEDURAL_REJECTION_MSG) is too broad and treats lone words like "update" or
"delete" in normal prose as SQL; narrow the heuristic by requiring SQL keywords
to appear in SQL-like contexts (e.g., "UPDATE <identifier>\s+SET",
"DELETE\s+FROM <identifier>", "INSERT\s+INTO <identifier>", "CREATE\s+TABLE
<identifier>", or a terminating semicolon) or by requiring a keyword followed by
an identifier or reserved token (FROM/SET/INTO); update the re.search pattern
accordingly to match those stricter forms against the content variable and keep
returning _PROCEDURAL_REJECTION_MSG only when the stricter pattern matches.
Owner
|
auto-review: changes requested. The PR did not pass these checks. Address each finding and push an amend / new commit on the same branch; the auto-reviewer respawns on the next dispatcher tick. Matrix checks (U1–U5, C1–C5)
Skill-PR checks (S1–S5)
Findings are mechanical (matrix) or judgment-based (role-reviewer). If a finding looks wrong, leave a counter-comment on the kanban task and Sahil will adjudicate on human-review. |
sahilm-ti
force-pushed
the
kanban/t_3877a824
branch
from
May 28, 2026 09:40
1908a83 to
cd430d8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/autonomous-ai-agents/hermes-agent/SKILL.md`:
- Line 1057: The docs confuse chars vs tokens: tools/memory_tool.py uses
memory_char_limit = 2200 (and user_char_limit = 1375) and _char_limit("memory")
returns that char limit, but the SKILL.md and docs show “~800” without units;
update the SKILL.md acceptance criterion and any docs/config references to
explicitly state units (e.g., change “≤800” to “≤~800 tokens” or to “≤800 chars”
consistently), and add a short parenthetical note clarifying that ~800 is a
token estimate for a 2200-character memory_char_limit; ensure references to
memory_char_limit, user_char_limit and _char_limit("memory") are consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0b72546f-676b-4494-a9fe-aed8bfab0e6c
📥 Commits
Reviewing files that changed from the base of the PR and between 1908a83c56b70b996c3a187ef0a52f4c6e23ae62 and cd430d8fec61ed40c12fb69a53fb80444db356a1.
📒 Files selected for processing (3)
skills/autonomous-ai-agents/hermes-agent/SKILL.mdtests/tools/test_memory_procedural_gate.pytools/memory_tool.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tools/memory_tool.py
- tests/tools/test_memory_procedural_gate.py
|
|
||
| Memory bloat follows a pattern: the tool gate is the hard layer, the skill rule is the soft | ||
| layer, and the weekly watchdog (cron job `memory-audit-watchdog`) catches drift that slips | ||
| through both. If memory reaches >70% of the 2200-char cap, the watchdog pings for a manual audit. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for memory char limit definitions
# Look for limit constants in memory_tool.py
rg -nP '\b(memory|user).*char.*limit\b' tools/memory_tool.py -C 2
# Search for default values or constants
rg -nP '(MEMORY|USER).*LIMIT|limit.*=.*\d{3,4}' tools/memory_tool.py -C 2
# Check if limits are configurable
rg -nP 'memory_char_limit|user_char_limit' tools/memory_tool.py -C 2Repository: sahilm-ti/hermes-agent
Length of output: 1777
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Focus on the SKILL.md for the stated acceptance criterion
SKILL="skills/autonomous-ai-agents/hermes-agent/SKILL.md"
test -f "$SKILL" && rg -n --context 2 "memory file.*800|≤ 800|800 chars|800-char|memory.*800" "$SKILL" || true
# Find other references to 2200/char cap and watchdog threshold text
test -f "$SKILL" && rg -n --context 2 "2200|watchdog|70%" "$SKILL" || true
# Search the codebase for 800 and memory file length/serialization caps
rg -n --context 2 "(memory file|memory.*file).* (800|800 chars|800-char|800-character)|\b800\b.*(char|chars)|limit.*800" -S . || true
# Inspect how memory_char_limit is actually used when producing the final memory file/prompt
rg -n --context 2 "memory_char_limit|_char_limit\\(|char_limit|memory.*entries|load_from_disk|write.*memory|dump.*memory" tools memory -S . || trueRepository: sahilm-ti/hermes-agent
Length of output: 50446
🏁 Script executed:
#!/bin/bash
set -euo pipefail
SKILL="skills/autonomous-ai-agents/hermes-agent/SKILL.md"
# 1) Confirm what the SKILL.md PR objective / acceptance criterion actually says about "800 chars"
if [ -f "$SKILL" ]; then
echo "== SKILL.md: occurrences of 800 around memory/cap/limit =="
rg -n --context 3 "memory|cap|limit|watchdog|audit" "$SKILL" | rg -n "800|2200|70%|0\.7" || true
echo "== SKILL.md: direct search for 'memory file' and '800' =="
rg -n --context 3 "memory file|memory.*file" "$SKILL" || true
rg -n --context 3 "800.*char|800 chars|≤ ?800|<= ?800" "$SKILL" || true
rg -n --context 3 "800.*token|~800 tokens|800 tokens" "$SKILL" || true
fi
# 2) Find watchdog threshold computation
echo "== Code: watchdog/70% computation candidates =="
rg -n --context 3 "memory-audit-watchdog|manual audit|70%|0\.7.*memory|memory_char_limit.*0\.7|0\.7.*memory_char_limit" -S . || true
# also inspect watchdog script/module if present
rg -n --context 3 "memory-audit-watchdog" -S . || trueRepository: sahilm-ti/hermes-agent
Length of output: 50380
Clarify unit: “800” is tokens, implementation uses 2200 chars
tools/memory_tool.pydefaultsmemory_char_limitto 2200 chars (anduser_char_limitto 1375);_char_limit("memory")returnsself.memory_char_limit.- The SKILL text (“>70% of the 2200-char cap”) matches that.
- The “~800” value shown in docs/config is a token estimate for
memory_char_limit: 2200; if the acceptance criterion says “≤800 chars”, it should be updated to “≤~800 tokens” (or explicitly clarify the unit).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/autonomous-ai-agents/hermes-agent/SKILL.md` at line 1057, The docs
confuse chars vs tokens: tools/memory_tool.py uses memory_char_limit = 2200 (and
user_char_limit = 1375) and _char_limit("memory") returns that char limit, but
the SKILL.md and docs show “~800” without units; update the SKILL.md acceptance
criterion and any docs/config references to explicitly state units (e.g., change
“≤800” to “≤~800 tokens” or to “≤800 chars” consistently), and add a short
parenthetical note clarifying that ~800 is a token estimate for a 2200-character
memory_char_limit; ensure references to memory_char_limit, user_char_limit and
_char_limit("memory") are consistent.
sahilm-ti
force-pushed
the
kanban/t_3877a824
branch
from
May 28, 2026 09:53
3bf3ee6 to
e3ac884
Compare
- memory_tool.py: _detect_procedural_content() with 4 heuristics (.md paths, SQL/code/shell, numbered steps, signal word near verb); wired into add() and replace() before size check; bypass_procedural_check=True param - tests/tools/test_memory_procedural_gate.py: 39 tests covering all 4 patterns and bypass flag - skills/autonomous-ai-agents/hermes-agent/SKILL.md: 'Memory - When NOT to use it' section with the 4 anti-patterns, bypass guidance, 3-layer-defence - scripts/release.py: add sahil.ai@ti.trilogy.com and 97122673+sahilm-ti@users.noreply.github.com to AUTHOR_MAP
sahilm-ti
force-pushed
the
kanban/t_3877a824
branch
from
May 28, 2026 10:01
7b78801 to
b58f85c
Compare
Owner
|
auto-review: approved, awaiting human merge + kanban_approve. Matrix checks (U1–U5, C1–C5, S1–S5)
Code-quality judgment (role-reviewer)Verdict: APPROVED Violations:
Architecture, DI, SOLID, fallback, security: no violations. _detect_procedural_content is a pure function, bypass_procedural_check=False default is secure (reject by default), no silent fallbacks on required values. |
sahilm-ti
pushed a commit
that referenced
this pull request
May 28, 2026
…ocs (#30) - memory_tool.py: _detect_procedural_content() with 4 heuristics (.md paths, SQL/code/shell, numbered steps, signal word near verb); wired into add() and replace() before size check; bypass_procedural_check=True param - tests/tools/test_memory_procedural_gate.py: 39 tests covering all 4 patterns and bypass flag - skills/autonomous-ai-agents/hermes-agent/SKILL.md: 'Memory - When NOT to use it' section with the 4 anti-patterns, bypass guidance, 3-layer-defence - scripts/release.py: add sahil.ai@ti.trilogy.com and 97122673+sahilm-ti@users.noreply.github.com to AUTHOR_MAP Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
May 28, 2026
…es, route findings to orchestrator
## Why
Today's auto-reviewer only fires on kanban_review (PR open). Cards that
complete via kanban_complete skip the review pass entirely. This led to
real misses:
- t_b0e9a537 (investigation): shipped findings in kanban_comment instead
of a Google Doc — no rule fired.
- t_aa450c9d (skill edit): category completeness not verified.
## What (Option B: separate sdlc-completion-audit skill)
Option B was chosen over A because the regime semantics diverge enough
(no retry, audit-only read-only pass, orchestrator-targeted verdict)
that folding into sdlc-review would add noisy conditionals throughout.
A separate skill keeps both reviewers readable.
### DB changes (hermes_cli/kanban_db.py)
- New column: tasks.completion_audit_at (INTEGER, NULL = no audit pending)
- Migration: _migrate_add_optional_columns adds the column + sparse index
- _maybe_schedule_completion_audit: sets the flag on kanban_complete when:
1. At least one real worker run (claimed event exists)
2. No GitHub PR URL in events/comments
3. No skip-review directive in body
4. Not already scheduled (idempotent)
- claim_completion_audit_task: atomically claims for audit (CAS on
completion_audit_at IS NOT NULL, task stays done)
- complete_completion_audit: closes audit run, emits completion_audit_done
event carrying the failed_rules list for repeat-offense detection
- dispatch_once: new completion-audit column dispatch loop that:
- Scans done tasks with completion_audit_at IS NOT NULL
- Claims, resolves workspace, spawns with skills=[sdlc-completion-audit]
- Re-arms trigger on workspace/spawn failures (retry next tick)
- Counts audit spawns against max_spawn
- Reports in DispatchResult.audited (task_id, assignee, workspace_path)
### Skill (profiles-level, ~/.hermes/skills/devops/sdlc-completion-audit/)
Ships separately from this PR (profiles-level skill, not bundled).
Task-class classifier: investigation / exploration / skill-edit /
memory-write / deliverable-doc / other (first-match keyword lookup).
Per-class rule sets: INV-1…5, EXP-1…4, SKL-1…3, MEM-1…2, DOC-1…2, OTH-1.
Repeat-offense detection: 3+ distinct cards failing same rule in 7 days
→ PATTERN ALERT prepended to the orchestrator comment.
## Verification
Historical smoke-test on 5 cards from the past 7 days:
| Card | Class | Expected audit | Actual schedule decision |
|---|---|---|---|
| t_b0e9a537 | investigation | YES (no PR, no skip) | scheduled=True ✓ |
| t_a83ff71d | investigation | YES (no PR, no skip) | scheduled=True ✓ |
| t_3877a824 | skill-edit/PR | NO (has PR #30) | scheduled=False ✓ |
| t_aa450c9d | skill-edit/PR | NO (has PR #35) | scheduled=False ✓ |
| t_0bc7806c | other | YES (no PR) | scheduled=True ✓ |
Lint verification on t_b0e9a537 (INV-1 expected to FAIL):
- INV-1: FAIL — no google Doc URL in summary or comments
- INV-4: PASS — summary has verdict (>50 chars)
Lint verification on t_a83ff71d (should PASS):
- INV-1: PASS — Doc URL in summary
- INV-4: PASS — summary has conclusion
PR flow unchanged: test_review_flow_unchanged_with_audit_present confirms
review-status tasks still spawn with skills=[sdlc-review]; audited list
is empty for those cards.
## Tests
- 18 new tests in tests/hermes_cli/test_kanban_completion_audit.py
- All 18 pass; 266 total (existing kanban_db suite) pass
- Acceptance criteria covered:
- Schema migration (test_schema_has_completion_audit_at)
- Schedule / no-schedule conditions (3 tests)
- Idempotency (test_completion_audit_scheduling_idempotent)
- Atomic claim + double-claim prevention (2 tests)
- claim returns None when not scheduled (test_claim_...not_scheduled)
- Run row created on claim (test_claim_...creates_run_row)
- complete_completion_audit releases claim, emits event (2 tests)
- dispatch_once dry-run, spawn, trigger-cleared, no-double-spawn (4 tests)
- max_spawn budgeting (test_dispatch_completion_audit_counts_toward_max_spawn)
- PR flow unchanged (2 tests)
sahilm-ti
pushed a commit
that referenced
this pull request
May 28, 2026
…es, route findings to orchestrator (#34) ## Why Today's auto-reviewer only fires on kanban_review (PR open). Cards that complete via kanban_complete skip the review pass entirely. This led to real misses: - t_b0e9a537 (investigation): shipped findings in kanban_comment instead of a Google Doc — no rule fired. - t_aa450c9d (skill edit): category completeness not verified. ## What (Option B: separate sdlc-completion-audit skill) Option B was chosen over A because the regime semantics diverge enough (no retry, audit-only read-only pass, orchestrator-targeted verdict) that folding into sdlc-review would add noisy conditionals throughout. A separate skill keeps both reviewers readable. ### DB changes (hermes_cli/kanban_db.py) - New column: tasks.completion_audit_at (INTEGER, NULL = no audit pending) - Migration: _migrate_add_optional_columns adds the column + sparse index - _maybe_schedule_completion_audit: sets the flag on kanban_complete when: 1. At least one real worker run (claimed event exists) 2. No GitHub PR URL in events/comments 3. No skip-review directive in body 4. Not already scheduled (idempotent) - claim_completion_audit_task: atomically claims for audit (CAS on completion_audit_at IS NOT NULL, task stays done) - complete_completion_audit: closes audit run, emits completion_audit_done event carrying the failed_rules list for repeat-offense detection - dispatch_once: new completion-audit column dispatch loop that: - Scans done tasks with completion_audit_at IS NOT NULL - Claims, resolves workspace, spawns with skills=[sdlc-completion-audit] - Re-arms trigger on workspace/spawn failures (retry next tick) - Counts audit spawns against max_spawn - Reports in DispatchResult.audited (task_id, assignee, workspace_path) ### Skill (profiles-level, ~/.hermes/skills/devops/sdlc-completion-audit/) Ships separately from this PR (profiles-level skill, not bundled). Task-class classifier: investigation / exploration / skill-edit / memory-write / deliverable-doc / other (first-match keyword lookup). Per-class rule sets: INV-1…5, EXP-1…4, SKL-1…3, MEM-1…2, DOC-1…2, OTH-1. Repeat-offense detection: 3+ distinct cards failing same rule in 7 days → PATTERN ALERT prepended to the orchestrator comment. ## Verification Historical smoke-test on 5 cards from the past 7 days: | Card | Class | Expected audit | Actual schedule decision | |---|---|---|---| | t_b0e9a537 | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_a83ff71d | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_3877a824 | skill-edit/PR | NO (has PR #30) | scheduled=False ✓ | | t_aa450c9d | skill-edit/PR | NO (has PR #35) | scheduled=False ✓ | | t_0bc7806c | other | YES (no PR) | scheduled=True ✓ | Lint verification on t_b0e9a537 (INV-1 expected to FAIL): - INV-1: FAIL — no google Doc URL in summary or comments - INV-4: PASS — summary has verdict (>50 chars) Lint verification on t_a83ff71d (should PASS): - INV-1: PASS — Doc URL in summary - INV-4: PASS — summary has conclusion PR flow unchanged: test_review_flow_unchanged_with_audit_present confirms review-status tasks still spawn with skills=[sdlc-review]; audited list is empty for those cards. ## Tests - 18 new tests in tests/hermes_cli/test_kanban_completion_audit.py - All 18 pass; 266 total (existing kanban_db suite) pass - Acceptance criteria covered: - Schema migration (test_schema_has_completion_audit_at) - Schedule / no-schedule conditions (3 tests) - Idempotency (test_completion_audit_scheduling_idempotent) - Atomic claim + double-claim prevention (2 tests) - claim returns None when not scheduled (test_claim_...not_scheduled) - Run row created on claim (test_claim_...creates_run_row) - complete_completion_audit releases claim, emits event (2 tests) - dispatch_once dry-run, spawn, trigger-cleared, no-double-spawn (4 tests) - max_spawn budgeting (test_dispatch_completion_audit_counts_toward_max_spawn) - PR flow unchanged (2 tests) Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
May 29, 2026
…ocs (#30) - memory_tool.py: _detect_procedural_content() with 4 heuristics (.md paths, SQL/code/shell, numbered steps, signal word near verb); wired into add() and replace() before size check; bypass_procedural_check=True param - tests/tools/test_memory_procedural_gate.py: 39 tests covering all 4 patterns and bypass flag - skills/autonomous-ai-agents/hermes-agent/SKILL.md: 'Memory - When NOT to use it' section with the 4 anti-patterns, bypass guidance, 3-layer-defence - scripts/release.py: add sahil.ai@ti.trilogy.com and 97122673+sahilm-ti@users.noreply.github.com to AUTHOR_MAP Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
May 29, 2026
…es, route findings to orchestrator (#34) Today's auto-reviewer only fires on kanban_review (PR open). Cards that complete via kanban_complete skip the review pass entirely. This led to real misses: - t_b0e9a537 (investigation): shipped findings in kanban_comment instead of a Google Doc — no rule fired. - t_aa450c9d (skill edit): category completeness not verified. Option B was chosen over A because the regime semantics diverge enough (no retry, audit-only read-only pass, orchestrator-targeted verdict) that folding into sdlc-review would add noisy conditionals throughout. A separate skill keeps both reviewers readable. - New column: tasks.completion_audit_at (INTEGER, NULL = no audit pending) - Migration: _migrate_add_optional_columns adds the column + sparse index - _maybe_schedule_completion_audit: sets the flag on kanban_complete when: 1. At least one real worker run (claimed event exists) 2. No GitHub PR URL in events/comments 3. No skip-review directive in body 4. Not already scheduled (idempotent) - claim_completion_audit_task: atomically claims for audit (CAS on completion_audit_at IS NOT NULL, task stays done) - complete_completion_audit: closes audit run, emits completion_audit_done event carrying the failed_rules list for repeat-offense detection - dispatch_once: new completion-audit column dispatch loop that: - Scans done tasks with completion_audit_at IS NOT NULL - Claims, resolves workspace, spawns with skills=[sdlc-completion-audit] - Re-arms trigger on workspace/spawn failures (retry next tick) - Counts audit spawns against max_spawn - Reports in DispatchResult.audited (task_id, assignee, workspace_path) Ships separately from this PR (profiles-level skill, not bundled). Task-class classifier: investigation / exploration / skill-edit / memory-write / deliverable-doc / other (first-match keyword lookup). Per-class rule sets: INV-1…5, EXP-1…4, SKL-1…3, MEM-1…2, DOC-1…2, OTH-1. Repeat-offense detection: 3+ distinct cards failing same rule in 7 days → PATTERN ALERT prepended to the orchestrator comment. Historical smoke-test on 5 cards from the past 7 days: | Card | Class | Expected audit | Actual schedule decision | |---|---|---|---| | t_b0e9a537 | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_a83ff71d | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_3877a824 | skill-edit/PR | NO (has PR #30) | scheduled=False ✓ | | t_aa450c9d | skill-edit/PR | NO (has PR #35) | scheduled=False ✓ | | t_0bc7806c | other | YES (no PR) | scheduled=True ✓ | Lint verification on t_b0e9a537 (INV-1 expected to FAIL): - INV-1: FAIL — no google Doc URL in summary or comments - INV-4: PASS — summary has verdict (>50 chars) Lint verification on t_a83ff71d (should PASS): - INV-1: PASS — Doc URL in summary - INV-4: PASS — summary has conclusion PR flow unchanged: test_review_flow_unchanged_with_audit_present confirms review-status tasks still spawn with skills=[sdlc-review]; audited list is empty for those cards. - 18 new tests in tests/hermes_cli/test_kanban_completion_audit.py - All 18 pass; 266 total (existing kanban_db suite) pass - Acceptance criteria covered: - Schema migration (test_schema_has_completion_audit_at) - Schedule / no-schedule conditions (3 tests) - Idempotency (test_completion_audit_scheduling_idempotent) - Atomic claim + double-claim prevention (2 tests) - claim returns None when not scheduled (test_claim_...not_scheduled) - Run row created on claim (test_claim_...creates_run_row) - complete_completion_audit releases claim, emits event (2 tests) - dispatch_once dry-run, spawn, trigger-cleared, no-double-spawn (4 tests) - max_spawn budgeting (test_dispatch_completion_audit_counts_toward_max_spawn) - PR flow unchanged (2 tests) Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 3, 2026
…ocs (#30) - memory_tool.py: _detect_procedural_content() with 4 heuristics (.md paths, SQL/code/shell, numbered steps, signal word near verb); wired into add() and replace() before size check; bypass_procedural_check=True param - tests/tools/test_memory_procedural_gate.py: 39 tests covering all 4 patterns and bypass flag - skills/autonomous-ai-agents/hermes-agent/SKILL.md: 'Memory - When NOT to use it' section with the 4 anti-patterns, bypass guidance, 3-layer-defence - scripts/release.py: add sahil.ai@ti.trilogy.com and 97122673+sahilm-ti@users.noreply.github.com to AUTHOR_MAP Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 3, 2026
…es, route findings to orchestrator (#34) Today's auto-reviewer only fires on kanban_review (PR open). Cards that complete via kanban_complete skip the review pass entirely. This led to real misses: - t_b0e9a537 (investigation): shipped findings in kanban_comment instead of a Google Doc — no rule fired. - t_aa450c9d (skill edit): category completeness not verified. Option B was chosen over A because the regime semantics diverge enough (no retry, audit-only read-only pass, orchestrator-targeted verdict) that folding into sdlc-review would add noisy conditionals throughout. A separate skill keeps both reviewers readable. - New column: tasks.completion_audit_at (INTEGER, NULL = no audit pending) - Migration: _migrate_add_optional_columns adds the column + sparse index - _maybe_schedule_completion_audit: sets the flag on kanban_complete when: 1. At least one real worker run (claimed event exists) 2. No GitHub PR URL in events/comments 3. No skip-review directive in body 4. Not already scheduled (idempotent) - claim_completion_audit_task: atomically claims for audit (CAS on completion_audit_at IS NOT NULL, task stays done) - complete_completion_audit: closes audit run, emits completion_audit_done event carrying the failed_rules list for repeat-offense detection - dispatch_once: new completion-audit column dispatch loop that: - Scans done tasks with completion_audit_at IS NOT NULL - Claims, resolves workspace, spawns with skills=[sdlc-completion-audit] - Re-arms trigger on workspace/spawn failures (retry next tick) - Counts audit spawns against max_spawn - Reports in DispatchResult.audited (task_id, assignee, workspace_path) Ships separately from this PR (profiles-level skill, not bundled). Task-class classifier: investigation / exploration / skill-edit / memory-write / deliverable-doc / other (first-match keyword lookup). Per-class rule sets: INV-1…5, EXP-1…4, SKL-1…3, MEM-1…2, DOC-1…2, OTH-1. Repeat-offense detection: 3+ distinct cards failing same rule in 7 days → PATTERN ALERT prepended to the orchestrator comment. Historical smoke-test on 5 cards from the past 7 days: | Card | Class | Expected audit | Actual schedule decision | |---|---|---|---| | t_b0e9a537 | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_a83ff71d | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_3877a824 | skill-edit/PR | NO (has PR #30) | scheduled=False ✓ | | t_aa450c9d | skill-edit/PR | NO (has PR #35) | scheduled=False ✓ | | t_0bc7806c | other | YES (no PR) | scheduled=True ✓ | Lint verification on t_b0e9a537 (INV-1 expected to FAIL): - INV-1: FAIL — no google Doc URL in summary or comments - INV-4: PASS — summary has verdict (>50 chars) Lint verification on t_a83ff71d (should PASS): - INV-1: PASS — Doc URL in summary - INV-4: PASS — summary has conclusion PR flow unchanged: test_review_flow_unchanged_with_audit_present confirms review-status tasks still spawn with skills=[sdlc-review]; audited list is empty for those cards. - 18 new tests in tests/hermes_cli/test_kanban_completion_audit.py - All 18 pass; 266 total (existing kanban_db suite) pass - Acceptance criteria covered: - Schema migration (test_schema_has_completion_audit_at) - Schedule / no-schedule conditions (3 tests) - Idempotency (test_completion_audit_scheduling_idempotent) - Atomic claim + double-claim prevention (2 tests) - claim returns None when not scheduled (test_claim_...not_scheduled) - Run row created on claim (test_claim_...creates_run_row) - complete_completion_audit releases claim, emits event (2 tests) - dispatch_once dry-run, spawn, trigger-cleared, no-double-spawn (4 tests) - max_spawn budgeting (test_dispatch_completion_audit_counts_toward_max_spawn) - PR flow unchanged (2 tests) Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 3, 2026
…es, route findings to orchestrator (#34) Today's auto-reviewer only fires on kanban_review (PR open). Cards that complete via kanban_complete skip the review pass entirely. This led to real misses: - t_b0e9a537 (investigation): shipped findings in kanban_comment instead of a Google Doc — no rule fired. - t_aa450c9d (skill edit): category completeness not verified. Option B was chosen over A because the regime semantics diverge enough (no retry, audit-only read-only pass, orchestrator-targeted verdict) that folding into sdlc-review would add noisy conditionals throughout. A separate skill keeps both reviewers readable. - New column: tasks.completion_audit_at (INTEGER, NULL = no audit pending) - Migration: _migrate_add_optional_columns adds the column + sparse index - _maybe_schedule_completion_audit: sets the flag on kanban_complete when: 1. At least one real worker run (claimed event exists) 2. No GitHub PR URL in events/comments 3. No skip-review directive in body 4. Not already scheduled (idempotent) - claim_completion_audit_task: atomically claims for audit (CAS on completion_audit_at IS NOT NULL, task stays done) - complete_completion_audit: closes audit run, emits completion_audit_done event carrying the failed_rules list for repeat-offense detection - dispatch_once: new completion-audit column dispatch loop that: - Scans done tasks with completion_audit_at IS NOT NULL - Claims, resolves workspace, spawns with skills=[sdlc-completion-audit] - Re-arms trigger on workspace/spawn failures (retry next tick) - Counts audit spawns against max_spawn - Reports in DispatchResult.audited (task_id, assignee, workspace_path) Ships separately from this PR (profiles-level skill, not bundled). Task-class classifier: investigation / exploration / skill-edit / memory-write / deliverable-doc / other (first-match keyword lookup). Per-class rule sets: INV-1…5, EXP-1…4, SKL-1…3, MEM-1…2, DOC-1…2, OTH-1. Repeat-offense detection: 3+ distinct cards failing same rule in 7 days → PATTERN ALERT prepended to the orchestrator comment. Historical smoke-test on 5 cards from the past 7 days: | Card | Class | Expected audit | Actual schedule decision | |---|---|---|---| | t_b0e9a537 | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_a83ff71d | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_3877a824 | skill-edit/PR | NO (has PR #30) | scheduled=False ✓ | | t_aa450c9d | skill-edit/PR | NO (has PR #35) | scheduled=False ✓ | | t_0bc7806c | other | YES (no PR) | scheduled=True ✓ | Lint verification on t_b0e9a537 (INV-1 expected to FAIL): - INV-1: FAIL — no google Doc URL in summary or comments - INV-4: PASS — summary has verdict (>50 chars) Lint verification on t_a83ff71d (should PASS): - INV-1: PASS — Doc URL in summary - INV-4: PASS — summary has conclusion PR flow unchanged: test_review_flow_unchanged_with_audit_present confirms review-status tasks still spawn with skills=[sdlc-review]; audited list is empty for those cards. - 18 new tests in tests/hermes_cli/test_kanban_completion_audit.py - All 18 pass; 266 total (existing kanban_db suite) pass - Acceptance criteria covered: - Schema migration (test_schema_has_completion_audit_at) - Schedule / no-schedule conditions (3 tests) - Idempotency (test_completion_audit_scheduling_idempotent) - Atomic claim + double-claim prevention (2 tests) - claim returns None when not scheduled (test_claim_...not_scheduled) - Run row created on claim (test_claim_...creates_run_row) - complete_completion_audit releases claim, emits event (2 tests) - dispatch_once dry-run, spawn, trigger-cleared, no-double-spawn (4 tests) - max_spawn budgeting (test_dispatch_completion_audit_counts_toward_max_spawn) - PR flow unchanged (2 tests) Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 9, 2026
…ocs (#30) - memory_tool.py: _detect_procedural_content() with 4 heuristics (.md paths, SQL/code/shell, numbered steps, signal word near verb); wired into add() and replace() before size check; bypass_procedural_check=True param - tests/tools/test_memory_procedural_gate.py: 39 tests covering all 4 patterns and bypass flag - skills/autonomous-ai-agents/hermes-agent/SKILL.md: 'Memory - When NOT to use it' section with the 4 anti-patterns, bypass guidance, 3-layer-defence - scripts/release.py: add sahil.ai@ti.trilogy.com and 97122673+sahilm-ti@users.noreply.github.com to AUTHOR_MAP Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 9, 2026
…es, route findings to orchestrator (#34) Today's auto-reviewer only fires on kanban_review (PR open). Cards that complete via kanban_complete skip the review pass entirely. This led to real misses: - t_b0e9a537 (investigation): shipped findings in kanban_comment instead of a Google Doc — no rule fired. - t_aa450c9d (skill edit): category completeness not verified. Option B was chosen over A because the regime semantics diverge enough (no retry, audit-only read-only pass, orchestrator-targeted verdict) that folding into sdlc-review would add noisy conditionals throughout. A separate skill keeps both reviewers readable. - New column: tasks.completion_audit_at (INTEGER, NULL = no audit pending) - Migration: _migrate_add_optional_columns adds the column + sparse index - _maybe_schedule_completion_audit: sets the flag on kanban_complete when: 1. At least one real worker run (claimed event exists) 2. No GitHub PR URL in events/comments 3. No skip-review directive in body 4. Not already scheduled (idempotent) - claim_completion_audit_task: atomically claims for audit (CAS on completion_audit_at IS NOT NULL, task stays done) - complete_completion_audit: closes audit run, emits completion_audit_done event carrying the failed_rules list for repeat-offense detection - dispatch_once: new completion-audit column dispatch loop that: - Scans done tasks with completion_audit_at IS NOT NULL - Claims, resolves workspace, spawns with skills=[sdlc-completion-audit] - Re-arms trigger on workspace/spawn failures (retry next tick) - Counts audit spawns against max_spawn - Reports in DispatchResult.audited (task_id, assignee, workspace_path) Ships separately from this PR (profiles-level skill, not bundled). Task-class classifier: investigation / exploration / skill-edit / memory-write / deliverable-doc / other (first-match keyword lookup). Per-class rule sets: INV-1…5, EXP-1…4, SKL-1…3, MEM-1…2, DOC-1…2, OTH-1. Repeat-offense detection: 3+ distinct cards failing same rule in 7 days → PATTERN ALERT prepended to the orchestrator comment. Historical smoke-test on 5 cards from the past 7 days: | Card | Class | Expected audit | Actual schedule decision | |---|---|---|---| | t_b0e9a537 | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_a83ff71d | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_3877a824 | skill-edit/PR | NO (has PR #30) | scheduled=False ✓ | | t_aa450c9d | skill-edit/PR | NO (has PR #35) | scheduled=False ✓ | | t_0bc7806c | other | YES (no PR) | scheduled=True ✓ | Lint verification on t_b0e9a537 (INV-1 expected to FAIL): - INV-1: FAIL — no google Doc URL in summary or comments - INV-4: PASS — summary has verdict (>50 chars) Lint verification on t_a83ff71d (should PASS): - INV-1: PASS — Doc URL in summary - INV-4: PASS — summary has conclusion PR flow unchanged: test_review_flow_unchanged_with_audit_present confirms review-status tasks still spawn with skills=[sdlc-review]; audited list is empty for those cards. - 18 new tests in tests/hermes_cli/test_kanban_completion_audit.py - All 18 pass; 266 total (existing kanban_db suite) pass - Acceptance criteria covered: - Schema migration (test_schema_has_completion_audit_at) - Schedule / no-schedule conditions (3 tests) - Idempotency (test_completion_audit_scheduling_idempotent) - Atomic claim + double-claim prevention (2 tests) - claim returns None when not scheduled (test_claim_...not_scheduled) - Run row created on claim (test_claim_...creates_run_row) - complete_completion_audit releases claim, emits event (2 tests) - dispatch_once dry-run, spawn, trigger-cleared, no-double-spawn (4 tests) - max_spawn budgeting (test_dispatch_completion_audit_counts_toward_max_spawn) - PR flow unchanged (2 tests) Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 10, 2026
…ocs (#30) - memory_tool.py: _detect_procedural_content() with 4 heuristics (.md paths, SQL/code/shell, numbered steps, signal word near verb); wired into add() and replace() before size check; bypass_procedural_check=True param - tests/tools/test_memory_procedural_gate.py: 39 tests covering all 4 patterns and bypass flag - skills/autonomous-ai-agents/hermes-agent/SKILL.md: 'Memory - When NOT to use it' section with the 4 anti-patterns, bypass guidance, 3-layer-defence - scripts/release.py: add sahil.ai@ti.trilogy.com and 97122673+sahilm-ti@users.noreply.github.com to AUTHOR_MAP Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 10, 2026
…es, route findings to orchestrator (#34) Today's auto-reviewer only fires on kanban_review (PR open). Cards that complete via kanban_complete skip the review pass entirely. This led to real misses: - t_b0e9a537 (investigation): shipped findings in kanban_comment instead of a Google Doc — no rule fired. - t_aa450c9d (skill edit): category completeness not verified. Option B was chosen over A because the regime semantics diverge enough (no retry, audit-only read-only pass, orchestrator-targeted verdict) that folding into sdlc-review would add noisy conditionals throughout. A separate skill keeps both reviewers readable. - New column: tasks.completion_audit_at (INTEGER, NULL = no audit pending) - Migration: _migrate_add_optional_columns adds the column + sparse index - _maybe_schedule_completion_audit: sets the flag on kanban_complete when: 1. At least one real worker run (claimed event exists) 2. No GitHub PR URL in events/comments 3. No skip-review directive in body 4. Not already scheduled (idempotent) - claim_completion_audit_task: atomically claims for audit (CAS on completion_audit_at IS NOT NULL, task stays done) - complete_completion_audit: closes audit run, emits completion_audit_done event carrying the failed_rules list for repeat-offense detection - dispatch_once: new completion-audit column dispatch loop that: - Scans done tasks with completion_audit_at IS NOT NULL - Claims, resolves workspace, spawns with skills=[sdlc-completion-audit] - Re-arms trigger on workspace/spawn failures (retry next tick) - Counts audit spawns against max_spawn - Reports in DispatchResult.audited (task_id, assignee, workspace_path) Ships separately from this PR (profiles-level skill, not bundled). Task-class classifier: investigation / exploration / skill-edit / memory-write / deliverable-doc / other (first-match keyword lookup). Per-class rule sets: INV-1…5, EXP-1…4, SKL-1…3, MEM-1…2, DOC-1…2, OTH-1. Repeat-offense detection: 3+ distinct cards failing same rule in 7 days → PATTERN ALERT prepended to the orchestrator comment. Historical smoke-test on 5 cards from the past 7 days: | Card | Class | Expected audit | Actual schedule decision | |---|---|---|---| | t_b0e9a537 | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_a83ff71d | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_3877a824 | skill-edit/PR | NO (has PR #30) | scheduled=False ✓ | | t_aa450c9d | skill-edit/PR | NO (has PR #35) | scheduled=False ✓ | | t_0bc7806c | other | YES (no PR) | scheduled=True ✓ | Lint verification on t_b0e9a537 (INV-1 expected to FAIL): - INV-1: FAIL — no google Doc URL in summary or comments - INV-4: PASS — summary has verdict (>50 chars) Lint verification on t_a83ff71d (should PASS): - INV-1: PASS — Doc URL in summary - INV-4: PASS — summary has conclusion PR flow unchanged: test_review_flow_unchanged_with_audit_present confirms review-status tasks still spawn with skills=[sdlc-review]; audited list is empty for those cards. - 18 new tests in tests/hermes_cli/test_kanban_completion_audit.py - All 18 pass; 266 total (existing kanban_db suite) pass - Acceptance criteria covered: - Schema migration (test_schema_has_completion_audit_at) - Schedule / no-schedule conditions (3 tests) - Idempotency (test_completion_audit_scheduling_idempotent) - Atomic claim + double-claim prevention (2 tests) - claim returns None when not scheduled (test_claim_...not_scheduled) - Run row created on claim (test_claim_...creates_run_row) - complete_completion_audit releases claim, emits event (2 tests) - dispatch_once dry-run, spawn, trigger-cleared, no-double-spawn (4 tests) - max_spawn budgeting (test_dispatch_completion_audit_counts_toward_max_spawn) - PR flow unchanged (2 tests) Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 11, 2026
…ocs (#30) - memory_tool.py: _detect_procedural_content() with 4 heuristics (.md paths, SQL/code/shell, numbered steps, signal word near verb); wired into add() and replace() before size check; bypass_procedural_check=True param - tests/tools/test_memory_procedural_gate.py: 39 tests covering all 4 patterns and bypass flag - skills/autonomous-ai-agents/hermes-agent/SKILL.md: 'Memory - When NOT to use it' section with the 4 anti-patterns, bypass guidance, 3-layer-defence - scripts/release.py: add sahil.ai@ti.trilogy.com and 97122673+sahilm-ti@users.noreply.github.com to AUTHOR_MAP Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 11, 2026
…es, route findings to orchestrator (#34) Today's auto-reviewer only fires on kanban_review (PR open). Cards that complete via kanban_complete skip the review pass entirely. This led to real misses: - t_b0e9a537 (investigation): shipped findings in kanban_comment instead of a Google Doc — no rule fired. - t_aa450c9d (skill edit): category completeness not verified. Option B was chosen over A because the regime semantics diverge enough (no retry, audit-only read-only pass, orchestrator-targeted verdict) that folding into sdlc-review would add noisy conditionals throughout. A separate skill keeps both reviewers readable. - New column: tasks.completion_audit_at (INTEGER, NULL = no audit pending) - Migration: _migrate_add_optional_columns adds the column + sparse index - _maybe_schedule_completion_audit: sets the flag on kanban_complete when: 1. At least one real worker run (claimed event exists) 2. No GitHub PR URL in events/comments 3. No skip-review directive in body 4. Not already scheduled (idempotent) - claim_completion_audit_task: atomically claims for audit (CAS on completion_audit_at IS NOT NULL, task stays done) - complete_completion_audit: closes audit run, emits completion_audit_done event carrying the failed_rules list for repeat-offense detection - dispatch_once: new completion-audit column dispatch loop that: - Scans done tasks with completion_audit_at IS NOT NULL - Claims, resolves workspace, spawns with skills=[sdlc-completion-audit] - Re-arms trigger on workspace/spawn failures (retry next tick) - Counts audit spawns against max_spawn - Reports in DispatchResult.audited (task_id, assignee, workspace_path) Ships separately from this PR (profiles-level skill, not bundled). Task-class classifier: investigation / exploration / skill-edit / memory-write / deliverable-doc / other (first-match keyword lookup). Per-class rule sets: INV-1…5, EXP-1…4, SKL-1…3, MEM-1…2, DOC-1…2, OTH-1. Repeat-offense detection: 3+ distinct cards failing same rule in 7 days → PATTERN ALERT prepended to the orchestrator comment. Historical smoke-test on 5 cards from the past 7 days: | Card | Class | Expected audit | Actual schedule decision | |---|---|---|---| | t_b0e9a537 | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_a83ff71d | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_3877a824 | skill-edit/PR | NO (has PR #30) | scheduled=False ✓ | | t_aa450c9d | skill-edit/PR | NO (has PR #35) | scheduled=False ✓ | | t_0bc7806c | other | YES (no PR) | scheduled=True ✓ | Lint verification on t_b0e9a537 (INV-1 expected to FAIL): - INV-1: FAIL — no google Doc URL in summary or comments - INV-4: PASS — summary has verdict (>50 chars) Lint verification on t_a83ff71d (should PASS): - INV-1: PASS — Doc URL in summary - INV-4: PASS — summary has conclusion PR flow unchanged: test_review_flow_unchanged_with_audit_present confirms review-status tasks still spawn with skills=[sdlc-review]; audited list is empty for those cards. - 18 new tests in tests/hermes_cli/test_kanban_completion_audit.py - All 18 pass; 266 total (existing kanban_db suite) pass - Acceptance criteria covered: - Schema migration (test_schema_has_completion_audit_at) - Schedule / no-schedule conditions (3 tests) - Idempotency (test_completion_audit_scheduling_idempotent) - Atomic claim + double-claim prevention (2 tests) - claim returns None when not scheduled (test_claim_...not_scheduled) - Run row created on claim (test_claim_...creates_run_row) - complete_completion_audit releases claim, emits event (2 tests) - dispatch_once dry-run, spawn, trigger-cleared, no-double-spawn (4 tests) - max_spawn budgeting (test_dispatch_completion_audit_counts_toward_max_spawn) - PR flow unchanged (2 tests) Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 13, 2026
…ocs (#30) - memory_tool.py: _detect_procedural_content() with 4 heuristics (.md paths, SQL/code/shell, numbered steps, signal word near verb); wired into add() and replace() before size check; bypass_procedural_check=True param - tests/tools/test_memory_procedural_gate.py: 39 tests covering all 4 patterns and bypass flag - skills/autonomous-ai-agents/hermes-agent/SKILL.md: 'Memory - When NOT to use it' section with the 4 anti-patterns, bypass guidance, 3-layer-defence - scripts/release.py: add sahil.ai@ti.trilogy.com and 97122673+sahilm-ti@users.noreply.github.com to AUTHOR_MAP Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 13, 2026
…es, route findings to orchestrator (#34) Today's auto-reviewer only fires on kanban_review (PR open). Cards that complete via kanban_complete skip the review pass entirely. This led to real misses: - t_b0e9a537 (investigation): shipped findings in kanban_comment instead of a Google Doc — no rule fired. - t_aa450c9d (skill edit): category completeness not verified. Option B was chosen over A because the regime semantics diverge enough (no retry, audit-only read-only pass, orchestrator-targeted verdict) that folding into sdlc-review would add noisy conditionals throughout. A separate skill keeps both reviewers readable. - New column: tasks.completion_audit_at (INTEGER, NULL = no audit pending) - Migration: _migrate_add_optional_columns adds the column + sparse index - _maybe_schedule_completion_audit: sets the flag on kanban_complete when: 1. At least one real worker run (claimed event exists) 2. No GitHub PR URL in events/comments 3. No skip-review directive in body 4. Not already scheduled (idempotent) - claim_completion_audit_task: atomically claims for audit (CAS on completion_audit_at IS NOT NULL, task stays done) - complete_completion_audit: closes audit run, emits completion_audit_done event carrying the failed_rules list for repeat-offense detection - dispatch_once: new completion-audit column dispatch loop that: - Scans done tasks with completion_audit_at IS NOT NULL - Claims, resolves workspace, spawns with skills=[sdlc-completion-audit] - Re-arms trigger on workspace/spawn failures (retry next tick) - Counts audit spawns against max_spawn - Reports in DispatchResult.audited (task_id, assignee, workspace_path) Ships separately from this PR (profiles-level skill, not bundled). Task-class classifier: investigation / exploration / skill-edit / memory-write / deliverable-doc / other (first-match keyword lookup). Per-class rule sets: INV-1…5, EXP-1…4, SKL-1…3, MEM-1…2, DOC-1…2, OTH-1. Repeat-offense detection: 3+ distinct cards failing same rule in 7 days → PATTERN ALERT prepended to the orchestrator comment. Historical smoke-test on 5 cards from the past 7 days: | Card | Class | Expected audit | Actual schedule decision | |---|---|---|---| | t_b0e9a537 | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_a83ff71d | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_3877a824 | skill-edit/PR | NO (has PR #30) | scheduled=False ✓ | | t_aa450c9d | skill-edit/PR | NO (has PR #35) | scheduled=False ✓ | | t_0bc7806c | other | YES (no PR) | scheduled=True ✓ | Lint verification on t_b0e9a537 (INV-1 expected to FAIL): - INV-1: FAIL — no google Doc URL in summary or comments - INV-4: PASS — summary has verdict (>50 chars) Lint verification on t_a83ff71d (should PASS): - INV-1: PASS — Doc URL in summary - INV-4: PASS — summary has conclusion PR flow unchanged: test_review_flow_unchanged_with_audit_present confirms review-status tasks still spawn with skills=[sdlc-review]; audited list is empty for those cards. - 18 new tests in tests/hermes_cli/test_kanban_completion_audit.py - All 18 pass; 266 total (existing kanban_db suite) pass - Acceptance criteria covered: - Schema migration (test_schema_has_completion_audit_at) - Schedule / no-schedule conditions (3 tests) - Idempotency (test_completion_audit_scheduling_idempotent) - Atomic claim + double-claim prevention (2 tests) - claim returns None when not scheduled (test_claim_...not_scheduled) - Run row created on claim (test_claim_...creates_run_row) - complete_completion_audit releases claim, emits event (2 tests) - dispatch_once dry-run, spawn, trigger-cleared, no-double-spawn (4 tests) - max_spawn budgeting (test_dispatch_completion_audit_counts_toward_max_spawn) - PR flow unchanged (2 tests) Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 15, 2026
…ocs (#30) - memory_tool.py: _detect_procedural_content() with 4 heuristics (.md paths, SQL/code/shell, numbered steps, signal word near verb); wired into add() and replace() before size check; bypass_procedural_check=True param - tests/tools/test_memory_procedural_gate.py: 39 tests covering all 4 patterns and bypass flag - skills/autonomous-ai-agents/hermes-agent/SKILL.md: 'Memory - When NOT to use it' section with the 4 anti-patterns, bypass guidance, 3-layer-defence - scripts/release.py: add sahil.ai@ti.trilogy.com and 97122673+sahilm-ti@users.noreply.github.com to AUTHOR_MAP Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 15, 2026
…es, route findings to orchestrator (#34) Today's auto-reviewer only fires on kanban_review (PR open). Cards that complete via kanban_complete skip the review pass entirely. This led to real misses: - t_b0e9a537 (investigation): shipped findings in kanban_comment instead of a Google Doc — no rule fired. - t_aa450c9d (skill edit): category completeness not verified. Option B was chosen over A because the regime semantics diverge enough (no retry, audit-only read-only pass, orchestrator-targeted verdict) that folding into sdlc-review would add noisy conditionals throughout. A separate skill keeps both reviewers readable. - New column: tasks.completion_audit_at (INTEGER, NULL = no audit pending) - Migration: _migrate_add_optional_columns adds the column + sparse index - _maybe_schedule_completion_audit: sets the flag on kanban_complete when: 1. At least one real worker run (claimed event exists) 2. No GitHub PR URL in events/comments 3. No skip-review directive in body 4. Not already scheduled (idempotent) - claim_completion_audit_task: atomically claims for audit (CAS on completion_audit_at IS NOT NULL, task stays done) - complete_completion_audit: closes audit run, emits completion_audit_done event carrying the failed_rules list for repeat-offense detection - dispatch_once: new completion-audit column dispatch loop that: - Scans done tasks with completion_audit_at IS NOT NULL - Claims, resolves workspace, spawns with skills=[sdlc-completion-audit] - Re-arms trigger on workspace/spawn failures (retry next tick) - Counts audit spawns against max_spawn - Reports in DispatchResult.audited (task_id, assignee, workspace_path) Ships separately from this PR (profiles-level skill, not bundled). Task-class classifier: investigation / exploration / skill-edit / memory-write / deliverable-doc / other (first-match keyword lookup). Per-class rule sets: INV-1…5, EXP-1…4, SKL-1…3, MEM-1…2, DOC-1…2, OTH-1. Repeat-offense detection: 3+ distinct cards failing same rule in 7 days → PATTERN ALERT prepended to the orchestrator comment. Historical smoke-test on 5 cards from the past 7 days: | Card | Class | Expected audit | Actual schedule decision | |---|---|---|---| | t_b0e9a537 | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_a83ff71d | investigation | YES (no PR, no skip) | scheduled=True ✓ | | t_3877a824 | skill-edit/PR | NO (has PR #30) | scheduled=False ✓ | | t_aa450c9d | skill-edit/PR | NO (has PR #35) | scheduled=False ✓ | | t_0bc7806c | other | YES (no PR) | scheduled=True ✓ | Lint verification on t_b0e9a537 (INV-1 expected to FAIL): - INV-1: FAIL — no google Doc URL in summary or comments - INV-4: PASS — summary has verdict (>50 chars) Lint verification on t_a83ff71d (should PASS): - INV-1: PASS — Doc URL in summary - INV-4: PASS — summary has conclusion PR flow unchanged: test_review_flow_unchanged_with_audit_present confirms review-status tasks still spawn with skills=[sdlc-review]; audited list is empty for those cards. - 18 new tests in tests/hermes_cli/test_kanban_completion_audit.py - All 18 pass; 266 total (existing kanban_db suite) pass - Acceptance criteria covered: - Schema migration (test_schema_has_completion_audit_at) - Schedule / no-schedule conditions (3 tests) - Idempotency (test_completion_audit_scheduling_idempotent) - Atomic claim + double-claim prevention (2 tests) - claim returns None when not scheduled (test_claim_...not_scheduled) - Run row created on claim (test_claim_...creates_run_row) - complete_completion_audit releases claim, emits event (2 tests) - dispatch_once dry-run, spawn, trigger-cleared, no-double-spawn (4 tests) - max_spawn budgeting (test_dispatch_completion_audit_counts_toward_max_spawn) - PR flow unchanged (2 tests) Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Stop the recurring memory-bloat pattern. The 2026-05-27 audit cleared
braintrustorch/memories/memory.mdto 0 bytes; 24 hours later it was back to 99% with procedural content. This PR fixes the behavior, not just the state.Changes
1.
tools/memory_tool.py— procedural-content gate_detect_procedural_content()function with 4 heuristics:/references/or ending in.mdgit,uv,hermes, etc.), or code blocks (```)1./2.or(1)/(2))via,use,run,recipe,procedure,flow:) within ±50 chars of an imperative verbskill_manageinstead of memorybypass_procedural_check=Trueparameter onadd(),replace(),memory_tool()dispatcher, OpenAI function schema, and registry handler — for legitimate env facts that happen to contain a path2.
tests/tools/test_memory_procedural_gate.py— 39 new tests3.
skills/autonomous-ai-agents/hermes-agent/SKILL.md— soft gatePart 1 (memory audit) done separately
braintrustorch/memories/memory.mdalready reduced to 480 chars (22% of cap) in-session before this PR.Part 2C (cron watchdog)
memory-audit-watchdogcron job registered (job375ed5e68acc,no_agent=True, weekly Monday 9am). Script at~/.hermes/profiles/braintrusteng/scripts/memory_audit_watchdog.sh. Successfully ran once (last run: ok, silent — all profiles within budget).Test results
111 tests pass (39 new + 72 existing memory tests).
Acceptance criteria
braintrustorch/memories/memory.md≤ 800 chars (480 chars)memory-audit-watchdogregistered and runs successfullySummary by CodeRabbit
New Features
Documentation
Tests