feat: add session-time-helper and risk field to task format#159
feat: add session-time-helper and risk field to task format#159marcusquinn merged 3 commits intomainfrom
Conversation
- Add session-time-helper.sh: analyses Claude Code session JSONL data to calculate active time (excluding AFK gaps > 5min threshold) Commands: list, analyse, summary, calibrate - Add risk:low/med/high field to TODO template (v1.1) indicating human oversight needed per task - Update time estimate semantics: now means active session time, not wall clock - Remove verbose (ai:Xh test:Xh) breakdown from template examples - Enhance upgrade-planning to detect TOON meta version changes (1.0 -> 1.1) so registered repos get offered the format upgrade - Simplify TOON schemas: remove est_ai/est_test/est_read columns, add risk
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 introduces a new utility for tracking active development time within Claude Code sessions, which helps in more accurate task estimation. It also refines the task management workflow by adding a 'risk' field to the TODO template, guiding the level of human oversight required for each task. Furthermore, the system's upgrade mechanism is improved to automatically detect and prompt for updates to these new template versions. Highlights
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
|
|
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. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntroduces session-time-helper.sh, a comprehensive Bash utility for analyzing Claude Code sessions with list, analyze, summary, and calibrate commands. Simultaneously updates the TODO.md template to incorporate risk management fields and reframe time estimates as active session duration, and enhances aidevops.sh with version-aware upgrade logic for planning files. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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. 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 |
🔍 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: Sat Jan 24 00:13:02 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
There was a problem hiding this comment.
Code Review
This pull request introduces a helpful session-time-helper.sh script for analyzing development time and updates the task format to include a risk field. The changes to aidevops.sh to support template versioning are also a good improvement.
My review focuses on improving the new shell script's robustness and maintainability. I've suggested refactoring the argument parsing in session-time-helper.sh to use a more standard and reliable pattern. I also found an inefficiency and a potential bug in the cmd_calibrate function related to parsing task data. Finally, I've pointed out some code duplication in aidevops.sh that could be addressed by creating a helper function.
Overall, these are great additions. The feedback provided should help make the new script more solid.
| main() { | ||
| # Separate command and session-id from options | ||
| local command="list" | ||
| local session_arg="" | ||
| local pass_through=() | ||
|
|
||
| for arg in "$@"; do | ||
| case "$arg" in | ||
| list|analyse|analyze|summary|calibrate|help|--help|-h) | ||
| command="$arg" | ||
| ;; | ||
| --project|--threshold|--limit) | ||
| pass_through+=("$arg") | ||
| ;; | ||
| --json) | ||
| OUTPUT_JSON=true | ||
| ;; | ||
| *) | ||
| # If previous arg was a flag expecting a value, it's the value | ||
| if [[ ${#pass_through[@]} -gt 0 ]]; then | ||
| local last="${pass_through[-1]}" | ||
| case "$last" in | ||
| --project) PROJECT_PATH="$arg" ;; | ||
| --threshold) AFK_THRESHOLD="$arg" ;; | ||
| --limit) LIMIT="$arg" ;; | ||
| *) session_arg="$arg" ;; | ||
| esac | ||
| # Remove the flag from pass_through since we consumed it | ||
| unset 'pass_through[-1]' | ||
| else | ||
| session_arg="$arg" | ||
| fi | ||
| ;; | ||
| esac | ||
| done | ||
|
|
||
| # Default project path to PWD | ||
| [[ -z "$PROJECT_PATH" ]] && PROJECT_PATH="$PWD" | ||
|
|
||
| case "$command" in | ||
| list) | ||
| cmd_list | ||
| ;; | ||
| analyse|analyze) | ||
| if [[ -z "$session_arg" ]]; then | ||
| echo -e "${RED}Usage: session-time-helper.sh analyse <session-id>${NC}" >&2 | ||
| return 1 | ||
| fi | ||
| cmd_analyse "$session_arg" | ||
| ;; | ||
| summary) | ||
| cmd_summary | ||
| ;; | ||
| calibrate) | ||
| cmd_calibrate | ||
| ;; | ||
| help|--help|-h) | ||
| head -19 "$0" | tail -17 | ||
| ;; | ||
| *) | ||
| echo -e "${RED}Unknown command: $command${NC}" >&2 | ||
| echo "Commands: list, analyse, summary, calibrate" >&2 | ||
| return 1 | ||
| ;; | ||
| esac | ||
| } |
There was a problem hiding this comment.
The argument parsing logic in the main function is overly complex and fragile. It uses a pass_through array to handle flags that require values, which can lead to incorrect behavior with certain combinations of arguments (e.g., ... --project --limit 10).
A more standard and robust approach in bash is to use a while loop with case and shift. This makes the code easier to read, maintain, and less prone to bugs.
main() {
local command="list"
local session_arg=""
local positional_args=()
while [[ $# -gt 0 ]]; do
case "$1" in
--project)
PROJECT_PATH="$2"; shift 2 ;;
--threshold)
AFK_THRESHOLD="$2"; shift 2 ;;
--limit)
LIMIT="$2"; shift 2 ;;
--json)
OUTPUT_JSON=true; shift ;;
-*)
echo -e "${RED}Unknown option: $1${NC}" >&2
return 1 ;;
*)
positional_args+=("$1")
shift ;;
esac
done
# Determine command and session_arg from positional args
for arg in "${positional_args[@]}"; do
case "$arg" in
list|analyse|analyze|summary|calibrate|help|--help|-h)
command="$arg" ;;
*)
session_arg="$arg" ;;
esac
done
# Default project path to PWD
[[ -z "$PROJECT_PATH" ]] && PROJECT_PATH="$PWD"
case "$command" in
list)
cmd_list
;;
analyse|analyze)
if [[ -z "$session_arg" ]]; then
echo -e "${RED}Usage: session-time-helper.sh analyse <session-id>${NC}" >&2
return 1
fi
cmd_analyse "$session_arg"
;;
summary)
cmd_summary
;;
calibrate)
cmd_calibrate
;;
help|--help|-h)
head -19 "$0" | tail -17
;;
*)
echo -e "${RED}Unknown command: $command${NC}" >&2
echo "Commands: list, analyse, summary, calibrate" >&2
return 1
;;
esac
}| if [[ "$line" =~ \[x\].*\ (t[0-9]+)\ (.+)~([0-9]+[hm][0-9]*[m]?) ]] && [[ "$line" =~ actual:([0-9]+[hm][0-9]*[m]?) ]]; then | ||
| local task_id="${BASH_REMATCH[1]}" | ||
| local estimate actual | ||
| # Re-extract with separate matches | ||
| estimate=$(echo "$line" | grep -o '~[0-9]*[hm][0-9]*[m]*' | head -1 | sed 's/~//') | ||
| actual=$(echo "$line" | grep -o 'actual:[0-9]*[hm][0-9]*[m]*' | head -1 | sed 's/actual://') | ||
| local title | ||
| title=$(echo "$line" | sed 's/.*\] //' | sed 's/ ~.*//' | cut -c1-35) |
There was a problem hiding this comment.
The logic to extract estimate and actual is inefficient because it re-parses the line with grep and sed after an initial regex match. A single, more comprehensive regex can capture all required values at once, making the code cleaner and more performant.
Additionally, there's a potential bug: the duration_to_minutes function does not handle floating-point hour values (e.g., 1.5h), which were present in the previous TODO.md template. While the new template uses minutes, this could cause incorrect calculations for older, completed tasks.
| if [[ "$line" =~ \[x\].*\ (t[0-9]+)\ (.+)~([0-9]+[hm][0-9]*[m]?) ]] && [[ "$line" =~ actual:([0-9]+[hm][0-9]*[m]?) ]]; then | |
| local task_id="${BASH_REMATCH[1]}" | |
| local estimate actual | |
| # Re-extract with separate matches | |
| estimate=$(echo "$line" | grep -o '~[0-9]*[hm][0-9]*[m]*' | head -1 | sed 's/~//') | |
| actual=$(echo "$line" | grep -o 'actual:[0-9]*[hm][0-9]*[m]*' | head -1 | sed 's/actual://') | |
| local title | |
| title=$(echo "$line" | sed 's/.*\] //' | sed 's/ ~.*//' | cut -c1-35) | |
| if [[ "$line" =~ ^-[[:space:]]*\[x\].*(t[0-9]+)[[:space:]].*~([0-9hm.]+).*actual:([0-9hm.]+) ]]; then | |
| local task_id="${BASH_REMATCH[1]}" | |
| local estimate="${BASH_REMATCH[2]}" | |
| local actual="${BASH_REMATCH[3]}" | |
| local title | |
| title=$(echo "$line" | sed 's/.*] //' | sed 's/ ~.*//' | cut -c1-35) |
aidevops.sh
Outdated
| # Check TOON meta version against template version | ||
| local current_version template_version | ||
| current_version=$(grep -A1 "TOON:meta" "$todo_file" 2>/dev/null | tail -1 | cut -d',' -f1) | ||
| template_version=$(grep -A1 "TOON:meta" "$todo_template" 2>/dev/null | tail -1 | cut -d',' -f1) | ||
| if [[ -n "$template_version" ]] && [[ "$current_version" != "$template_version" ]]; then | ||
| todo_needs_upgrade=true | ||
| needs_upgrade=true | ||
| print_warning "TODO.md format version $current_version -> $template_version (adds risk field, updated estimates)" | ||
| else | ||
| print_success "TODO.md already up to date (v${current_version})" | ||
| fi |
There was a problem hiding this comment.
The logic for checking the TOON meta version is duplicated for TODO.md (here) and todo/PLANS.md (lines 1153-1163). This code duplication makes the script harder to maintain. If the version checking logic needs to be updated in the future, it will have to be changed in two places.
Consider refactoring this logic into a helper function. This function could take the file path, template path, and a descriptive name as arguments, and it would handle the version comparison and printing messages. This would make cmd_upgrade_planning cleaner and more maintainable.
🤖 Augment PR SummarySummary: Adds tooling to measure Claude Code “active” development time (excluding AFK gaps) and updates the planning/TODO format to include a risk/oversight signal. Changes:
Technical Notes: The new script supports GNU 🤖 Was this summary useful? React with 👍 or 👎 |
| # Extract timestamps from a session JSONL file (user + assistant messages only) | ||
| extract_timestamps() { | ||
| local session_file="$1" | ||
| grep -o '"timestamp":"[^"]*"' "$session_file" 2>/dev/null \ |
| while IFS= read -r line; do | ||
| # Match: - [x] tNNN description ~Xh ... actual:Xh | ||
| if [[ "$line" =~ \[x\].*\ (t[0-9]+)\ (.+)~([0-9]+[hm][0-9]*[m]?) ]] && [[ "$line" =~ actual:([0-9]+[hm][0-9]*[m]?) ]]; then | ||
| local task_id="${BASH_REMATCH[1]}" |
| local dur="$1" | ||
| local minutes=0 | ||
|
|
||
| if [[ "$dur" =~ ([0-9]+)h ]]; then |
| if [[ "$OUTPUT_JSON" == "true" ]]; then | ||
| [[ "$first" == "true" ]] && first=false || echo "," | ||
| printf ' {"session_id":"%s","started":"%s","active_seconds":%d,"wall_seconds":%d,"messages":%d,"afk_seconds":%d,"title":"%s"}' \ | ||
| "$session_id" "$first_ts" "$active" "$wall" "$msgs" "$afk" "${title//\"/\\\"}" |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.agent/scripts/session-time-helper.sh:
- Around line 112-118: The extract_timestamps function currently greps all
"timestamp" occurrences; change it to filter only message objects with role
"user" or "assistant" before extracting timestamps (e.g., first grep for
'"role":"user"' or '"role":"assistant"' against the session_file, then extract
the '"timestamp":"..."' values and sort -u). Update the pipeline inside
extract_timestamps to operate on "$session_file" and preserve existing error
redirection while ensuring only user/assistant timestamps are returned.
- Around line 41-67: In get_sessions_dir, the git worktree lookup (the
main_worktree assignment using git -C "$project_path" worktree list) can fail
under set -e and abort the script; make this a best-effort by guarding the git
call so non-zero exit doesn't trigger script exit—e.g., temporarily disable
errexit or append a safe fallback (capture output with a
guarded/git-failing-safe construct) around the main_worktree assignment, then
proceed to check main_worktree as before; ensure you restore errexit behavior if
you change it and keep variable names main_worktree and sessions_dir unchanged.
- Around line 207-273: The JSON emitter in cmd_list currently only escapes
quotes in the title, which allows backslashes and control characters (newlines,
tabs, CRs) to produce invalid JSON; update the JSON branch inside cmd_list
(where printf ' {"session_id":...,"title":"%s"}' is used) to sanitize the title
variable by escaping backslashes and quotes and replacing control chars with
their JSON escape sequences (e.g., replace \ with \\\\, " with \\", newline ->
\\n, carriage return -> \\r, tab -> \\t); implement the transformation before
the printf (e.g., create an escaped_title from title) so the printed JSON is
always valid.
In `@aidevops.sh`:
- Around line 1127-1137: Replace the separate local declaration and later
assignment with inline local assignments for both TODO and PLANS version checks:
declare and assign current_version and template_version in one step using the
pattern local current_version="$(grep -A1 "TOON:meta" "$todo_file" 2>/dev/null |
tail -1 | cut -d',' -f1)" and local template_version="$(grep -A1 "TOON:meta"
"$todo_template" 2>/dev/null | tail -1 | cut -d',' -f1)" (preserve quoting and
redirection), and apply the same change to the PLANS block (use the
corresponding variables used there), leaving the rest of the logic
(todo_needs_upgrade/needs_upgrade, print_warning/print_success) unchanged.
🧹 Nitpick comments (1)
.agent/scripts/session-time-helper.sh (1)
120-177: Align locals + explicit returns with shell guidelines.
Use inline local assignments and add an explicitreturn 0at the end of helper functions; apply the same pattern across this script.♻️ Suggested adjustment (example)
- local timestamps - timestamps=$(extract_timestamps "$session_file") + local timestamps=$(extract_timestamps "$session_file") @@ - echo "${total_active}|${total_wall}|${msg_count}|${afk_time}|${first_ts}|${last_ts}" + echo "${total_active}|${total_wall}|${msg_count}|${afk_time}|${first_ts}|${last_ts}" + return 0As per coding guidelines, prefer inline local assignments and explicit returns in shell scripts.
aidevops.sh
Outdated
| # Check TOON meta version against template version | ||
| local current_version template_version | ||
| current_version=$(grep -A1 "TOON:meta" "$todo_file" 2>/dev/null | tail -1 | cut -d',' -f1) | ||
| template_version=$(grep -A1 "TOON:meta" "$todo_template" 2>/dev/null | tail -1 | cut -d',' -f1) | ||
| if [[ -n "$template_version" ]] && [[ "$current_version" != "$template_version" ]]; then | ||
| todo_needs_upgrade=true | ||
| needs_upgrade=true | ||
| print_warning "TODO.md format version $current_version -> $template_version (adds risk field, updated estimates)" | ||
| else | ||
| print_success "TODO.md already up to date (v${current_version})" | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and check the file size
wc -l aidevops.shRepository: marcusquinn/aidevops
Length of output: 80
🏁 Script executed:
# Read the specific line ranges mentioned in the review
echo "=== Lines 1127-1137 (TODO version check) ==="
sed -n '1127,1137p' aidevops.sh
echo ""
echo "=== Lines 1153-1163 (PLANS version check) ==="
sed -n '1153,1163p' aidevops.shRepository: marcusquinn/aidevops
Length of output: 1681
🏁 Script executed:
# Run ShellCheck on the file to identify any violations
shellcheck aidevops.sh 2>&1 | head -50Repository: marcusquinn/aidevops
Length of output: 46
🏁 Script executed:
# Check if the version extraction actually includes whitespace
# by examining what the grep + tail + cut pipeline returns
cat << 'TESTEOF' > /tmp/test_version.sh
#!/bin/bash
# Simulate the version extraction
test_line="1,Some,Data"
echo "Raw: '$test_line'"
echo "After cut: '$(echo "$test_line" | cut -d',' -f1)'"
echo "After cut+xargs: '$(echo "$test_line" | cut -d',' -f1 | xargs)'"
# Test with leading/trailing whitespace (simulating potential CRLF issues)
test_line_space=" 1 "
echo "Raw with spaces: '$test_line_space'"
echo "After xargs: '$(echo "$test_line_space" | xargs)'"
TESTEOF
bash /tmp/test_version.shRepository: marcusquinn/aidevops
Length of output: 160
Apply inline local assignments to both TODO and PLANS version checks.
Per shell coding guidelines, use local var="$(..." pattern instead of separate declaration and assignment. Both blocks (lines 1127–1137 and 1153–1163) should be updated for consistency.
- local current_version template_version
- current_version=$(grep -A1 "TOON:meta" "$todo_file" 2>/dev/null | tail -1 | cut -d',' -f1)
- template_version=$(grep -A1 "TOON:meta" "$todo_template" 2>/dev/null | tail -1 | cut -d',' -f1)
+ local current_version=$(grep -A1 "TOON:meta" "$todo_file" 2>/dev/null | tail -1 | cut -d',' -f1 | xargs)
+ local template_version=$(grep -A1 "TOON:meta" "$todo_template" 2>/dev/null | tail -1 | cut -d',' -f1 | xargs)Same pattern applies to lines 1153–1163 for PLANS.md version checks.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Check TOON meta version against template version | |
| local current_version template_version | |
| current_version=$(grep -A1 "TOON:meta" "$todo_file" 2>/dev/null | tail -1 | cut -d',' -f1) | |
| template_version=$(grep -A1 "TOON:meta" "$todo_template" 2>/dev/null | tail -1 | cut -d',' -f1) | |
| if [[ -n "$template_version" ]] && [[ "$current_version" != "$template_version" ]]; then | |
| todo_needs_upgrade=true | |
| needs_upgrade=true | |
| print_warning "TODO.md format version $current_version -> $template_version (adds risk field, updated estimates)" | |
| else | |
| print_success "TODO.md already up to date (v${current_version})" | |
| fi | |
| # Check TOON meta version against template version | |
| local current_version=$(grep -A1 "TOON:meta" "$todo_file" 2>/dev/null | tail -1 | cut -d',' -f1 | xargs) | |
| local template_version=$(grep -A1 "TOON:meta" "$todo_template" 2>/dev/null | tail -1 | cut -d',' -f1 | xargs) | |
| if [[ -n "$template_version" ]] && [[ "$current_version" != "$template_version" ]]; then | |
| todo_needs_upgrade=true | |
| needs_upgrade=true | |
| print_warning "TODO.md format version $current_version -> $template_version (adds risk field, updated estimates)" | |
| else | |
| print_success "TODO.md already up to date (v${current_version})" | |
| fi |
🤖 Prompt for AI Agents
In `@aidevops.sh` around lines 1127 - 1137, Replace the separate local declaration
and later assignment with inline local assignments for both TODO and PLANS
version checks: declare and assign current_version and template_version in one
step using the pattern local current_version="$(grep -A1 "TOON:meta"
"$todo_file" 2>/dev/null | tail -1 | cut -d',' -f1)" and local
template_version="$(grep -A1 "TOON:meta" "$todo_template" 2>/dev/null | tail -1
| cut -d',' -f1)" (preserve quoting and redirection), and apply the same change
to the PLANS block (use the corresponding variables used there), leaving the
rest of the logic (todo_needs_upgrade/needs_upgrade,
print_warning/print_success) unchanged.
- Rewrite arg parser: simple while/shift instead of fragile pass_through array - Fix BASH_REMATCH clobbering: use grep extraction instead of dual regex match - Filter extract_timestamps to only user/assistant messages (skip snapshots) - Handle decimal durations (1.5h -> 90m) in duration_to_minutes - Fix grep patterns to match decimals in estimates/actuals (e.g., actual:1.5h) - Add json_escape helper for proper JSON string escaping - Add || true guards in calibrate to prevent set -e exits on grep misses - Extract check_planning_file_version() to deduplicate aidevops.sh logic
🔍 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: Sat Jan 24 00:27:29 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
🔍 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: Sat Jan 24 00:30:00 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
All issues addressed in subsequent commits



Summary
session-time-helper.shto analyse Claude Code session active time (excluding AFK gaps)risk:low/med/highfield to TODO template indicating human oversight needed per taskupgrade-planningto detect TOON format version changes in registered reposChanges
New:
session-time-helper.shAnalyses
~/.claude/projects/JSONL session data to calculate active development time by measuring gaps between messages. Gaps > 5min (configurable) are excluded as AFK time.Commands:
list- Recent sessions with active vs wall timeanalyse <id>- Detailed breakdown with gap distributionsummary- Aggregate stats across all sessionscalibrate- Compare TODO.md estimates vs actuals (shows overestimate ratio)Updated: TODO template (v1.0 → v1.1)
risk:low/med/highfield (maps to autonomous/supervised/engaged oversight)(ai:Xh test:Xh read:Xm)breakdownriskcolumnUpdated:
aidevops.shupgrade-planningTesting
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.