feat: enhance TODO.md parser with full field extraction and rich issue body (t020.1)#804
feat: enhance TODO.md parser with full field extraction and rich issue body (t020.1)#804marcusquinn merged 2 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe issue-sync-helper.sh script is enhanced to extract and propagate additional TODO.md metadata fields (assignee, started, completed, actual, blocked_by, blocks, verified). These fields are parsed from task lines, composed into enriched GitHub issue bodies with a structured two-line metadata header, and synchronized through push/enrich/close workflows to maintain assignee and status consistency. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Summary of ChangesHello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Mon Feb 9 20:21:25 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the TODO.md parser by extracting several new fields and creating a richer issue body for GitHub synchronization. The changes also include important portability fixes by replacing \s with [[:space:]] in sed commands for macOS compatibility.
My review focuses on improving the efficiency of the parsing logic. While the new functionality is correct, the implementation introduces a large number of external process calls (grep, sed, cut) within loops, which can be very inefficient, especially when parsing large TODO.md files. I've provided suggestions to refactor these sections using shell built-ins like while read loops and regex matching, which will significantly improve performance and maintainability. I also identified a variable scope bug in cmd_push that could lead to incorrect issue titles.
| # Extract assignee | ||
| local assignee | ||
| assignee=$(echo "$line" | grep -oE 'assignee:[A-Za-z0-9._@-]+' | head -1 | sed 's/assignee://' || echo "") | ||
|
|
||
| # Extract started timestamp | ||
| local started | ||
| started=$(echo "$line" | grep -oE 'started:[0-9T:Z-]+' | head -1 | sed 's/started://' || echo "") | ||
|
|
||
| # Extract completed date | ||
| local completed | ||
| completed=$(echo "$line" | grep -oE 'completed:[0-9-]+' | head -1 | sed 's/completed://' || echo "") | ||
|
|
||
| # Extract actual time | ||
| local actual | ||
| actual=$(echo "$line" | grep -oE 'actual:[0-9.]+[hmd]' | head -1 | sed 's/actual://' || echo "") | ||
|
|
||
| # Extract blocked-by dependencies | ||
| local blocked_by | ||
| blocked_by=$(echo "$line" | grep -oE 'blocked-by:[A-Za-z0-9.,]+' | head -1 | sed 's/blocked-by://' || echo "") | ||
|
|
||
| # Extract blocks (downstream dependencies) | ||
| local blocks | ||
| blocks=$(echo "$line" | grep -oE 'blocks:[A-Za-z0-9.,]+' | head -1 | sed 's/blocks://' || echo "") | ||
|
|
||
| # Extract verified date | ||
| local verified | ||
| verified=$(echo "$line" | grep -oE 'verified:[0-9-]+' | head -1 | sed 's/verified://' || echo "") |
There was a problem hiding this comment.
While this correctly extracts the new fields, the approach of using echo | grep | head | sed for each field is highly inefficient. For each task line parsed, this will spawn 28 new processes just for these 7 fields. This can significantly slow down the script, especially with a large TODO.md file.
A much more performant approach is to use Bash's built-in regex matching. This avoids creating any external processes for parsing.
# Extract assignee
local assignee=""
if [[ "$line" =~ assignee:([A-Za-z0-9._@-]+) ]]; then
assignee="${BASH_REMATCH[1]}"
fi
# Extract started timestamp
local started=""
if [[ "$line" =~ started:([0-9T:Z-]+) ]]; then
started="${BASH_REMATCH[1]}"
fi
# Extract completed date
local completed=""
if [[ "$line" =~ completed:([0-9-]+) ]]; then
completed="${BASH_REMATCH[1]}"
fi
# Extract actual time
local actual=""
if [[ "$line" =~ actual:([0-9.]+[hmd]) ]]; then
actual="${BASH_REMATCH[1]}"
fi
# Extract blocked-by dependencies
local blocked_by=""
if [[ "$line" =~ blocked-by:([A-Za-z0-9.,]+) ]]; then
blocked_by="${BASH_REMATCH[1]}"
fi
# Extract blocks (downstream dependencies)
local blocks=""
if [[ "$line" =~ blocks:([A-Za-z0-9.,]+) ]]; then
blocks="${BASH_REMATCH[1]}"
fi
# Extract verified date
local verified=""
if [[ "$line" =~ verified:([0-9-]+) ]]; then
verified="${BASH_REMATCH[1]}"
fi| local description tags estimate plan_link status logged | ||
| local assignee started completed actual blocked_by blocks verified | ||
| description=$(echo "$parsed" | grep '^description=' | cut -d= -f2-) | ||
| tags=$(echo "$parsed" | grep '^tags=' | cut -d= -f2-) | ||
| estimate=$(echo "$parsed" | grep '^estimate=' | cut -d= -f2-) | ||
| plan_link=$(echo "$parsed" | grep '^plan_link=' | cut -d= -f2-) | ||
| status=$(echo "$parsed" | grep '^status=' | cut -d= -f2-) | ||
| logged=$(echo "$parsed" | grep '^logged=' | cut -d= -f2-) | ||
| assignee=$(echo "$parsed" | grep '^assignee=' | cut -d= -f2-) | ||
| started=$(echo "$parsed" | grep '^started=' | cut -d= -f2-) | ||
| completed=$(echo "$parsed" | grep '^completed=' | cut -d= -f2-) | ||
| actual=$(echo "$parsed" | grep '^actual=' | cut -d= -f2-) | ||
| blocked_by=$(echo "$parsed" | grep '^blocked_by=' | cut -d= -f2-) | ||
| blocks=$(echo "$parsed" | grep '^blocks=' | cut -d= -f2-) | ||
| verified=$(echo "$parsed" | grep '^verified=' | cut -d= -f2-) |
There was a problem hiding this comment.
Parsing the key=value output from parse_task_line by repeatedly calling echo | grep | cut for each of the 13+ fields is inefficient. It spawns multiple processes for every single field.
A more efficient and maintainable way to parse this is to use a single while read loop. This reads all variables in one go without spawning any external processes.
| local description tags estimate plan_link status logged | |
| local assignee started completed actual blocked_by blocks verified | |
| description=$(echo "$parsed" | grep '^description=' | cut -d= -f2-) | |
| tags=$(echo "$parsed" | grep '^tags=' | cut -d= -f2-) | |
| estimate=$(echo "$parsed" | grep '^estimate=' | cut -d= -f2-) | |
| plan_link=$(echo "$parsed" | grep '^plan_link=' | cut -d= -f2-) | |
| status=$(echo "$parsed" | grep '^status=' | cut -d= -f2-) | |
| logged=$(echo "$parsed" | grep '^logged=' | cut -d= -f2-) | |
| assignee=$(echo "$parsed" | grep '^assignee=' | cut -d= -f2-) | |
| started=$(echo "$parsed" | grep '^started=' | cut -d= -f2-) | |
| completed=$(echo "$parsed" | grep '^completed=' | cut -d= -f2-) | |
| actual=$(echo "$parsed" | grep '^actual=' | cut -d= -f2-) | |
| blocked_by=$(echo "$parsed" | grep '^blocked_by=' | cut -d= -f2-) | |
| blocks=$(echo "$parsed" | grep '^blocks=' | cut -d= -f2-) | |
| verified=$(echo "$parsed" | grep '^verified=' | cut -d= -f2-) | |
| local description="" tags="" estimate="" plan_link="" status="" logged="" | |
| local assignee="" started="" completed="" actual="" blocked_by="" blocks="" verified="" | |
| while IFS='=' read -r key value; do | |
| case "$key" in | |
| description) description="$value" ;; | |
| tags) tags="$value" ;; | |
| estimate) estimate="$value" ;; | |
| plan_link) plan_link="$value" ;; | |
| status) status="$value" ;; | |
| logged) logged="$value" ;; | |
| assignee) assignee="$value" ;; | |
| started) started="$value" ;; | |
| completed) completed="$value" ;; | |
| actual) actual="$value" ;; | |
| blocked_by) blocked_by="$value" ;; | |
| blocks) blocks="$value" ;; | |
| verified) verified="$value" ;; | |
| esac | |
| done <<< "$parsed" |
| description=$(echo "$parsed" | grep '^description=' | cut -d= -f2-) | ||
| local tags | ||
| tags=$(echo "$parsed" | grep '^tags=' | cut -d= -f2-) | ||
| local assignee | ||
| assignee=$(echo "$parsed" | grep '^assignee=' | cut -d= -f2-) |
There was a problem hiding this comment.
This block has two issues:
- Bug: The
descriptionvariable is not declared aslocalwithin the loop. This can cause it to retain its value from a previous iteration if a task has no description, leading to incorrect issue titles. - Inefficiency: The same inefficient parsing pattern of
echo | grep | cutis used here.
I suggest declaring all variables as local and using a while read loop for efficient and correct parsing.
| description=$(echo "$parsed" | grep '^description=' | cut -d= -f2-) | |
| local tags | |
| tags=$(echo "$parsed" | grep '^tags=' | cut -d= -f2-) | |
| local assignee | |
| assignee=$(echo "$parsed" | grep '^assignee=' | cut -d= -f2-) | |
| local description="" tags="" assignee="" | |
| while IFS='=' read -r key value; do | |
| case "$key" in | |
| description) description="$value" ;; | |
| tags) tags="$value" ;; | |
| assignee) assignee="$value" ;; | |
| esac | |
| done <<< "$parsed" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.agents/scripts/issue-sync-helper.sh:
- Line 670: The sed substitution producing gh_line for declined subtasks leaves
an opening "~~" but never closes it; update the transformation that handles "^-
\[-\]" (the sed fragment applied when building gh_line from subtask_line) so it
wraps the subtask text in matching "~~...~~" (i.e., produce "- [x]
~~<original-subtask-text>~~") rather than just prefixing "~~"; ensure the
replacement captures the rest of the line and inserts the closing "~~" after
that text so declined items render as strikethrough on GitHub.
🧹 Nitpick comments (2)
.agents/scripts/issue-sync-helper.sh (2)
1066-1081:sed_inplacereplacement with unescaped$new_lineis fragile with special characters.On Line 1078,
sed_inplace "${line_num}s|.*|${new_line}|"injects the raw TODO.md line content into asedsubstitution. If the line contains|(the delimiter),&(sed's match-backreference), or\, the substitution will produce corrupt output or fail silently. This is a pre-existing pattern in the file, but this new code path adds another instance.Consider escaping sed-special characters in
$new_linebefore substitution, or using a different approach (e.g.,awkwithNR==line_numfor line replacement).🛡️ Proposed fix — escape sed-special characters
+ # Escape sed-special chars in replacement string + local escaped_line + escaped_line=$(printf '%s\n' "$new_line" | sed -e 's/[&\|/]/\\&/g') - sed_inplace "${line_num}s|.*|${new_line}|" "$todo_file" + sed_inplace "${line_num}s|.*|${escaped_line}|" "$todo_file"Or, for a more robust alternative:
# Use awk for exact line replacement (no escaping needed) awk -v ln="$line_num" -v replacement="$new_line" 'NR==ln{print replacement; next}1' "$todo_file" > "${todo_file}.tmp" && mv "${todo_file}.tmp" "$todo_file"
922-938: Consider syncing assignee duringenrichas well.
cmd_pushsets the GitHub issue assignee via--assignee(Line 849), andcmd_pullsyncs assignees back to TODO.md (Line 1027+). However,cmd_enrichonly updates the issue body — if theassignee:field changes in TODO.md after initial creation, runningenrichwon't update the actual GitHub assignee. The body will display the updated@mention, but the issue's assigned user remains stale.If bi-directional assignee consistency is a goal, consider adding a
gh issue edit --add-assigneecall in the enrich flow when the parsed assignee differs from the current GitHub assignee.
The sed substitution for declined subtasks (- [-]) was adding opening ~~ but never closing it, producing broken strikethrough on GitHub. Now captures the rest of the line and wraps it in matching ~~...~~. Addresses CodeRabbit review feedback on PR #804.
…e body (t020.1) - Add 7 new parsed fields: assignee, started, completed, actual, blocked_by, blocks, verified - Fix description extraction for indented subtask lines (use POSIX [[:space:]] instead of \s for macOS sed compatibility) - Fix sed \s portability in extract_notes, extract_plan_section, and subtask rendering - Enhance compose_issue_body with status, assignee @mention, dates, and dependency metadata - Add assignee auto-assignment when pushing issues via cmd_push - All fields render in structured GitHub issue body with clear metadata sections
The sed substitution for declined subtasks (- [-]) was adding opening ~~ but never closing it, producing broken strikethrough on GitHub. Now captures the rest of the line and wraps it in matching ~~...~~. Addresses CodeRabbit review feedback on PR #804.
marcusquinn
force-pushed
the
feature/t020.1
branch
from
a13817c to
0434f81
Compare
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Mon Feb 9 20:46:44 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
Mark as done with PR references: - t020 Issue Sync Enrichment (6/6 subtasks, pr:#804) - t109 Parallel Agents & Headless Dispatch (5/5 subtasks, pr:#348) - t135 Code Quality Hardening (59/59 subtasks, pr:#422) - t199 Content Creation Agent Architecture (11/11 subtasks, pr:#840) - t236 Higgsfield Automator Production Hardening (6/6 subtasks, pr:#965) - t245 Unified Quality Debt Sweep (4/4 subtasks, pr:#984)
1 participant
Summary
parse_task_line()to extract 7 new fields:assignee,started,completed,actual,blocked_by,blocks,verified[[:space:]]instead of\sfor macOSsedcompatibility\sportability across 3 othersedcalls (extract_notes,extract_plan_section, subtask rendering)compose_issue_body()with structured metadata: status, assignee @mention, date fields, dependency linkscmd_pushTesting
Verified against real TODO.md data with
issue-sync-helper.sh parse:t020.1t001t001.1blocked-bydependencyt001.2blocksdependencyt002actualtime andcompleteddatet003[-]checkbox)t135ShellCheck: zero warnings/errors (only pre-existing SC1091 info + SC2016 false positive).
Task
t020.1: Build core TODO.md parser + rich issue body composer in issue-sync-helper.shSummary by CodeRabbit