Skip to content

t1284: fix create_improvement dedup window + add recent fixes context #2040

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.

Sign up for GitHub

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
marcusquinn merged 1 commit into from
Feb 21, 2026
Merged

t1284: fix create_improvement dedup window + add recent fixes context #2040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 22 additions & 12 deletions .agents/scripts/supervisor/ai-actions.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ AI_SEMANTIC_DEDUP_USE_AI="${AI_SEMANTIC_DEDUP_USE_AI:-true}"
# AI semantic dedup timeout in seconds
AI_SEMANTIC_DEDUP_TIMEOUT="${AI_SEMANTIC_DEDUP_TIMEOUT:-30}"

# create_improvement dedup: how many days back to scan completed tasks (t1284)
# The default 24h window misses fixes completed more than a day ago, causing the
# supervisor to re-create improvement tasks for already-fixed issues (e.g., t1284
# was created 9 days after t314 fixed the same worker hang problem).
# 30 days covers the typical fix→observe→re-trigger cycle for systemic issues.
AI_IMPROVEMENT_DEDUP_DAYS="${AI_IMPROVEMENT_DEDUP_DAYS:-30}"

# Valid action types — any action not in this list is rejected
readonly AI_VALID_ACTION_TYPES="comment_on_issue create_task create_subtasks flag_for_review adjust_priority close_verified request_info create_improvement escalate_model propose_auto_dispatch"

Expand Down Expand Up @@ -432,10 +439,10 @@ _keyword_prefilter_open_tasks() {
fi

# Collect candidates: task_id|match_count|title_excerpt
# Scan BOTH open tasks AND recently completed tasks (last 24h).
# Recently completed tasks matter because the same symptom shouldn't
# trigger a new investigation if one was just completed — the fix
# either worked (wait and observe) or didn't (the existing task
# Scan BOTH open tasks AND recently completed tasks (within AI_IMPROVEMENT_DEDUP_DAYS).
# t1284: Extended from 24h to 30 days — recently completed tasks matter because
# the same symptom shouldn't trigger a new investigation if a fix was already
# merged. The fix either worked (wait and observe) or didn't (the existing task
# should be reopened, not duplicated).
local candidates=""
local task_line
Expand Down Expand Up @@ -472,17 +479,20 @@ _keyword_prefilter_open_tasks() {
_score_task_line "$task_line"
done < <(grep -E '^\s*- \[ \] t[0-9]' "$todo_file" 2>/dev/null)

# Scan recently completed tasks (completed: field within last 24h)
# These are [x] tasks with a completed: timestamp from today or yesterday
local today yesterday
today=$(date -u '+%Y-%m-%d')
yesterday=$(date -u -v-1d '+%Y-%m-%d' 2>/dev/null || date -u -d 'yesterday' '+%Y-%m-%d' 2>/dev/null || echo "")
if [[ -n "$today" ]]; then
# Scan recently completed tasks (completed: field within AI_IMPROVEMENT_DEDUP_DAYS)
# t1284: Extended from 24h to 30 days (configurable via AI_IMPROVEMENT_DEDUP_DAYS).
# The 24h window missed fixes completed more than a day ago, causing the supervisor
# to re-create improvement tasks for already-fixed issues (e.g., t1284 was created
# 9 days after t314 fixed the same worker hang problem).
local dedup_days="${AI_IMPROVEMENT_DEDUP_DAYS:-30}"
local cutoff_date
cutoff_date=$(date -u -v-"${dedup_days}d" '+%Y-%m-%d' 2>/dev/null || date -u -d "${dedup_days} days ago" '+%Y-%m-%d' 2>/dev/null || echo "")
if [[ -n "$cutoff_date" ]]; then
while IFS= read -r task_line; do
# Only include if completed: timestamp is recent (today or yesterday)
# Only include if completed: timestamp is within the dedup window
local completed_date
completed_date=$(printf '%s' "$task_line" | grep -oE 'completed:[0-9]{4}-[0-9]{2}-[0-9]{2}' | head -1 | sed 's/completed://')
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

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

high

In a script with set -euo pipefail enabled, this command substitution will cause the script to exit if grep fails to find a match (returning exit code 1). Since the completed: field is optional or may not match the pattern, a || true guard is required to ensure the script continues. Additionally, use tail -1 instead of head -1 for robustness when extracting metadata fields, as the field might appear multiple times or be at the end of the line.

Suggested change
completed_date=$(printf '%s' "$task_line" | grep -oE 'completed:[0-9]{4}-[0-9]{2}-[0-9]{2}' | head -1 | sed 's/completed://')
completed_date=$(printf '%s' "$task_line" | grep -oE 'completed:[0-9]{4}-[0-9]{2}-[0-9]{2}' | tail -1 | sed 's/completed://' || true)
References
  1. Use || true guards for commands that may fail under set -e (grep, arithmetic) (link)
  2. In shell scripts with 'set -e' enabled, use '|| true' to prevent the script from exiting when a command like 'jq' fails on an optional lookup.
  3. When extracting metadata fields from a line using 'grep' and 'sed' in shell scripts, use 'tail -1' instead of 'head -1' to ensure robustness, especially when the field might appear multiple times or be at the end of the line.

if [[ "$completed_date" == "$today" || "$completed_date" == "$yesterday" ]]; then
if [[ -n "$completed_date" && "$completed_date" > "$cutoff_date" ]]; then
_score_task_line "$task_line"
fi
done < <(grep -E '^\s*- \[x\] t[0-9]' "$todo_file" 2>/dev/null)
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

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

medium

The use of 2>/dev/null here is redundant because the existence of $todo_file is verified at the start of the function. Furthermore, it violates the style guide's restriction on blanket error suppression. An explicit || true guard should be used instead to handle the case where no completed tasks are found.

Suggested change
done < <(grep -E '^\s*- \[x\] t[0-9]' "$todo_file" 2>/dev/null)
done < <(grep -E '^\s*- \[x\] t[0-9]' "$todo_file" || true)
References
  1. 2>/dev/null is acceptable ONLY when redirecting to log files, not blanket suppression (link)
  2. Avoid using 2>/dev/null to suppress errors on file operations if the file's existence has already been verified by a preceding check.

Expand Down
88 changes: 88 additions & 0 deletions .agents/scripts/supervisor/ai-context.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,15 @@ build_ai_context() {
context+="$(build_auto_dispatch_eligibility_context "$repo_path")\n\n"
fi

# Section 12: Recently Fixed Systemic Issues (t1284)
# Shows improvement/bugfix tasks completed in the last 30 days so the AI
# reasoner doesn't re-create investigation tasks for already-fixed problems.
# The create_improvement dedup only checks 24h; this section provides broader
# awareness of recent fixes to prevent stale-data false positives.
if [[ "$scope" == "full" ]]; then
context+="$(build_recent_fixes_context "$repo_path")\n\n"
fi
Comment on lines +104 to +111
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 21, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Stale comment: dedup window is no longer 24h.

Lines 107–108 state "The create_improvement dedup only checks 24h" — but this very PR updates _keyword_prefilter_open_tasks in ai-actions.sh to use AI_IMPROVEMENT_DEDUP_DAYS (default 30 days). The comment is now inaccurate.

Suggested fix 
 	# Section 12: Recently Fixed Systemic Issues (t1284)
 	# Shows improvement/bugfix tasks completed in the last 30 days so the AI
 	# reasoner doesn't re-create investigation tasks for already-fixed problems.
-	# The create_improvement dedup only checks 24h; this section provides broader
-	# awareness of recent fixes to prevent stale-data false positives.
+	# Complements the create_improvement dedup window (AI_IMPROVEMENT_DEDUP_DAYS)
+	# by giving the AI reasoner visibility into recent fixes before proposing actions.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.agents/scripts/supervisor/ai-context.sh around lines 104 - 111, Update the
stale inline comment in ai-context.sh (the lines describing the
create_improvement dedup window) to reflect that dedup is no longer fixed at 24h
but uses the configurable AI_IMPROVEMENT_DEDUP_DAYS (default 30 days) as
implemented in _keyword_prefilter_open_tasks in ai-actions.sh; change the
wording to mention the configurable dedup window and/or default 30 days so the
comment accurately matches the code behavior.


printf '%b' "$context"
return 0
}
Expand Down Expand Up @@ -1476,6 +1485,85 @@ build_auto_dispatch_eligibility_context() {
return 0
}

#######################################
# Section 12: Recently Fixed Systemic Issues (t1284)
# Shows improvement/bugfix tasks completed in the last 30 days so the AI
# reasoner doesn't re-create investigation tasks for already-fixed problems.
# The create_improvement dedup only checks 24h; this section provides broader
# awareness of recent fixes to prevent stale-data false positives.
#
# Arguments:
# $1 - repo path
# Outputs:
# Markdown section to stdout
#######################################
build_recent_fixes_context() {
local repo_path="${1:-$REPO_PATH}"
local dedup_days="${AI_IMPROVEMENT_DEDUP_DAYS:-30}"
local output="## Recently Fixed Systemic Issues (last ${dedup_days}d)\n\n"
output+="**IMPORTANT**: Do NOT create \`create_improvement\` tasks for problems listed here — they were already fixed.\n"
output+="If the same symptom recurs, check whether the fix was effective before proposing new work.\n\n"

local todo_file="${repo_path}/TODO.md"
if [[ ! -f "$todo_file" ]]; then
output+="*TODO.md not found*\n"
printf '%b' "$output"
return 0
fi
Comment on lines +1500 to +1512
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 21, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

build_recent_fixes_context only scans the primary repo — misses fixes in other registered repos.

build_ai_context passes only $repo_path (the primary repo) on line 110. However, Section 3 (lines 50–65) iterates over all registered repos from the supervisor DB to build TODO context. A systemic fix completed in a secondary repo's TODO.md won't appear in Section 12, creating a dedup blind spot — the exact class of problem this PR aims to fix.

Consider iterating over all registered repos the same way Section 3 does:

Sketch: multi-repo support

In build_ai_context (around line 110):

 	if [[ "$scope" == "full" ]]; then
-		context+="$(build_recent_fixes_context "$repo_path")\n\n"
+		# Include recent fixes from all registered repos (like Section 3)
+		local _fixes_ctx=""
+		if [[ -n "$all_context_repos" ]]; then
+			while IFS= read -r _fix_repo; do
+				[[ -z "$_fix_repo" || ! -d "$_fix_repo" ]] && continue
+				_fixes_ctx+="$(build_recent_fixes_context "$_fix_repo")\n\n"
+			done <<<"$all_context_repos"
+			if ! echo "$all_context_repos" | grep -qF "$repo_path"; then
+				_fixes_ctx+="$(build_recent_fixes_context "$repo_path")\n\n"
+			fi
+		else
+			_fixes_ctx="$(build_recent_fixes_context "$repo_path")\n\n"
+		fi
+		context+="$_fixes_ctx"
 	fi

Note: $all_context_repos is already populated on line 51 and would need to be accessible here (it's currently in scope since build_ai_context is one function).

Also applies to: 104-111

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

In @.agents/scripts/supervisor/ai-context.sh around lines 1500 - 1512,
build_recent_fixes_context currently only scans the primary repo; update callers
in build_ai_context to iterate over all registered repos (use the existing
all_context_repos variable) and invoke build_recent_fixes_context for each repo,
aggregating their outputs into Section 12 so TODO.md entries from secondary
repos are included; ensure build_recent_fixes_context still accepts a repo_path
parameter (defaulting to REPO_PATH) and preserve dedup_days handling when called
multiple times.


# Compute cutoff date (30 days ago)
local cutoff_date
cutoff_date=$(date -u -v-"${dedup_days}d" '+%Y-%m-%d' 2>/dev/null || date -u -d "${dedup_days} days ago" '+%Y-%m-%d' 2>/dev/null || echo "")

if [[ -z "$cutoff_date" ]]; then
output+="*Could not compute cutoff date*\n"
printf '%b' "$output"
return 0
fi

# Scan completed tasks with #bugfix, #self-improvement, or improvement-related keywords
local found_fixes=""
local fix_count=0
while IFS= read -r task_line; do
# Only include if completed: timestamp is within the dedup window
local completed_date
completed_date=$(printf '%s' "$task_line" | grep -oE 'completed:[0-9]{4}-[0-9]{2}-[0-9]{2}' | head -1 | sed 's/completed://')
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

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

high

This pipeline will trigger a script exit under set -e and pipefail if grep returns no matches. Add || true to allow the script to proceed with an empty completed_date variable, which is already handled by the subsequent null check. Additionally, use tail -1 instead of head -1 for robustness when extracting metadata fields, as the field might appear multiple times or be at the end of the line.

Suggested change
completed_date=$(printf '%s' "$task_line" | grep -oE 'completed:[0-9]{4}-[0-9]{2}-[0-9]{2}' | head -1 | sed 's/completed://')
completed_date=$(printf '%s' "$task_line" | grep -oE 'completed:[0-9]{4}-[0-9]{2}-[0-9]{2}' | tail -1 | sed 's/completed://' || true)
References
  1. Use || true guards for commands that may fail under set -e (grep, arithmetic) (link)
  2. When extracting metadata fields from a line using 'grep' and 'sed' in shell scripts, use 'tail -1' instead of 'head -1' to ensure robustness, especially when the field might appear multiple times or be at the end of the line.

if [[ -z "$completed_date" || ! "$completed_date" > "$cutoff_date" ]]; then
continue
fi

# Only include bugfix, self-improvement, or automation tasks
local lower_line
lower_line=$(printf '%s' "$task_line" | tr '[:upper:]' '[:lower:]')
if ! printf '%s' "$lower_line" | grep -qE '#bugfix|#self-improvement|#automation|fix|investigate|hang|timeout|worker|supervisor|dispatch'; then
continue
fi

local task_id
task_id=$(printf '%s' "$task_line" | grep -oE 't[0-9]+(\.[0-9]+)?' | head -1)
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

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

high

Similar to the completed_date assignment, this grep operation needs a || true guard to prevent the script from crashing if a task line somehow lacks a valid ID pattern. Additionally, use tail -1 instead of head -1 for robustness when extracting metadata fields, as the field might appear multiple times or be at the end of the line.

Suggested change
task_id=$(printf '%s' "$task_line" | grep -oE 't[0-9]+(\.[0-9]+)?' | head -1)
task_id=$(printf '%s' "$task_line" | grep -oE 't[0-9]+(\.[0-9]+)?' | tail -1 || true)
References
  1. Use || true guards for commands that may fail under set -e (grep, arithmetic) (link)
  2. When extracting metadata fields from a line using 'grep' and 'sed' in shell scripts, use 'tail -1' instead of 'head -1' to ensure robustness, especially when the field might appear multiple times or be at the end of the line.

[[ -z "$task_id" ]] && continue

local pr_ref
pr_ref=$(printf '%s' "$task_line" | grep -oE 'pr:#[0-9]+' | head -1 || echo "")

local excerpt
excerpt=$(printf '%s' "$task_line" | sed -E 's/^[[:space:]]*- \[x\] t[0-9]+(\.[0-9]+)? //' | head -c 120)

found_fixes+="- **$task_id** (completed: $completed_date${pr_ref:+, $pr_ref}): $excerpt\n"
fix_count=$((fix_count + 1))
done < <(grep -E '^\s*- \[x\] t[0-9]' "$todo_file" 2>/dev/null)
Copy link

@gemini-code-assist gemini-code-assist bot Feb 21, 2026

Choose a reason for hiding this comment

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

medium

Remove the redundant 2>/dev/null suppression and add an explicit || true guard to handle cases where no completed tasks match the pattern, ensuring compliance with the style guide and robustness under set -e.

Suggested change
done < <(grep -E '^\s*- \[x\] t[0-9]' "$todo_file" 2>/dev/null)
done < <(grep -E '^\s*- \[x\] t[0-9]' "$todo_file" || true)
References
  1. 2>/dev/null is acceptable ONLY when redirecting to log files, not blanket suppression (link)
  2. Avoid using 2>/dev/null to suppress errors on file operations if the file's existence has already been verified by a preceding check.


if [[ "$fix_count" -eq 0 ]]; then
output+="No systemic fixes found in the last ${dedup_days} days.\n"
else
output+="$fix_count fix(es) merged in the last ${dedup_days} days:\n\n"
output+="$(printf '%b' "$found_fixes")"
fi

printf '%b' "$output"
return 0
}

#######################################
# CLI entry point for standalone testing
# Usage: ai-context.sh [--scope full|quick] [--repo /path]
Expand Down
Loading