Release digest skill improvements - #1597
Conversation
WalkthroughThe release digest script now supports ChangesRelease digest baseline selection
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![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 introduces --pre-release and --since command-line options to the release-digest script, allowing users to customize the baseline tag for generating change digests. The documentation in SKILL.md has been updated to reflect these new options. The feedback suggests handling the edge case where --pre-release is used but no prior tag exists in the repository to avoid an unhandled git error.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if args.pre_release: | ||
| # The newest tag is a pre-release still being QA'd; baseline is the tag before | ||
| # it, so the range covers exactly what the pre-release contains. | ||
| prior = run(["git", "describe", "--tags", "--abbrev=0", f"{latest}^"]) |
There was a problem hiding this comment.
If latest is the only tag in the repository, running git describe on latest^ will fail because there is no prior tag. Using the helper run here will cause the script to exit with a generic git error message. It would be more user-friendly to handle this case gracefully and print a clear error message.
| prior = run(["git", "describe", "--tags", "--abbrev=0", f"{latest}^"]) | |
| prior_result = subprocess.run( | |
| ["git", "describe", "--tags", "--abbrev=0", f"{latest}^"], | |
| capture_output=True, | |
| text=True, | |
| ) | |
| if prior_result.returncode != 0: | |
| sys.stderr.write( | |
| f"ERROR: --pre-release mode failed because no prior tag could be found before {latest}.\n" | |
| ) | |
| sys.exit(1) | |
| prior = prior_result.stdout.strip() |
📊 Coverage ReportOverall Coverage: 92% Diff: origin/main...HEADNo lines with coverage information in this diff.
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.agents/skills/release-digest/SKILL.md (1)
198-200: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the checklist to reflect conditional prompting.
The checklist item "Asked the user for the new release name" conflicts with the updated Phase 3 instructions, which explicitly state that no prompting is needed in
--pre-releasemode. Updating this item will prevent the agent from getting confused or inappropriately blocking on user input.📝 Proposed fix
- - [ ] Asked the user for the new release name (used in the title) + - [ ] Determined the new release name (from pre-release tag, or asked the user)🤖 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 @.agents/skills/release-digest/SKILL.md around lines 198 - 200, Update the release-digest checklist item for asking the user for a new release name to reflect that prompting is required only when not running in --pre-release mode, matching the conditional behavior described in Phase 3.
🤖 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 @.agents/skills/release-digest/scripts/gather_changes.py:
- Around line 109-137: Update resolve_base_tag to accept a head_ref parameter,
handle and validate args.since before discovering the latest tag, and pass
head_ref to all git describe calls so tag discovery follows the fetched remote
reference rather than local HEAD. In main(), resolve head_ref before evaluating
last_tag and pass it into resolve_base_tag; preserve pre-release behavior by
finding the prior tag relative to the selected head_ref.
---
Outside diff comments:
In @.agents/skills/release-digest/SKILL.md:
- Around line 198-200: Update the release-digest checklist item for asking the
user for a new release name to reflect that prompting is required only when not
running in --pre-release mode, matching the conditional behavior described in
Phase 3.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 465ef6da-dab9-4820-84eb-a6a90a282f56
📒 Files selected for processing (2)
.agents/skills/release-digest/SKILL.md.agents/skills/release-digest/scripts/gather_changes.py
| def resolve_base_tag(args: argparse.Namespace) -> str: | ||
| """Pick the tag to compare against, honoring --since / --pre-release overrides. | ||
|
|
||
| Default: the newest tag reachable from HEAD (`git describe`). But when the newest | ||
| tag is a just-cut pre-release (main is already even with it), that baseline yields | ||
| an empty range -- so `--pre-release` steps back one tag to recap what went INTO the | ||
| pre-release, and `--since TAG` lets the caller name the baseline explicitly. | ||
| """ | ||
| latest = run(["git", "describe", "--tags", "--abbrev=0"]) | ||
| if args.since: | ||
| check = subprocess.run( | ||
| ["git", "rev-parse", "--verify", "--quiet", f"{args.since}^{{commit}}"], | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| if check.returncode != 0: | ||
| sys.stderr.write(f"ERROR: --since tag {args.since!r} does not exist.\n") | ||
| sys.exit(1) | ||
| return args.since | ||
| if args.pre_release: | ||
| # The newest tag is a pre-release still being QA'd; baseline is the tag before | ||
| # it, so the range covers exactly what the pre-release contains. | ||
| prior = run(["git", "describe", "--tags", "--abbrev=0", f"{latest}^"]) | ||
| sys.stderr.write( | ||
| f"Pre-release mode: newest tag {latest} treated as the release being " | ||
| f"QA'd; comparing against the prior tag {prior}.\n" | ||
| ) | ||
| return prior | ||
| return latest |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use head_ref for tag discovery and evaluate --since before latest.
There are two issues with the current resolve_base_tag implementation:
git describedefaults to the localHEAD. Becausegit fetchupdatesorigin/mainbut not your local branch, any newly tagged releases onoriginwon't be seen if your local branch is behind, leading to an incorrect (inflated) change set.latestis computed unconditionally. If the current branch has no reachable tags, the script will crash even if the caller explicitly provided a valid--sincetag.
Determine head_ref first, pass it to resolve_base_tag, and defer computing latest until after --since is handled.
🐛 Proposed fixes
Update resolve_base_tag to accept head_ref and reorder the logic:
-def resolve_base_tag(args: argparse.Namespace) -> str:
+def resolve_base_tag(args: argparse.Namespace, head_ref: str) -> str:
"""Pick the tag to compare against, honoring --since / --pre-release overrides.
- Default: the newest tag reachable from HEAD (`git describe`). But when the newest
+ Default: the newest tag reachable from the target branch. But when the newest
tag is a just-cut pre-release (main is already even with it), that baseline yields
an empty range -- so `--pre-release` steps back one tag to recap what went INTO the
pre-release, and `--since TAG` lets the caller name the baseline explicitly.
"""
- latest = run(["git", "describe", "--tags", "--abbrev=0"])
if args.since:
check = subprocess.run(
["git", "rev-parse", "--verify", "--quiet", f"{args.since}^{{commit}}"],
capture_output=True,
text=True,
)
if check.returncode != 0:
sys.stderr.write(f"ERROR: --since tag {args.since!r} does not exist.\n")
sys.exit(1)
return args.since
+ latest = run(["git", "describe", "--tags", "--abbrev=0", head_ref])
if args.pre_release:
# The newest tag is a pre-release still being QA'd; baseline is the tag before
# it, so the range covers exactly what the pre-release contains.
prior = run(["git", "describe", "--tags", "--abbrev=0", f"{latest}^"])
sys.stderr.write(
f"Pre-release mode: newest tag {latest} treated as the release being "
f"QA'd; comparing against the prior tag {prior}.\n"
)
return prior
return latestThen in main(), resolve head_ref before evaluating last_tag:
- last_tag = resolve_base_tag(args)
-
# Prefer origin/main; fall back to local main if there's no remote-tracking ref.
head_ref = "origin/main"
check = subprocess.run(
["git", "rev-parse", "--verify", "--quiet", head_ref],
capture_output=True,
text=True,
)
if check.returncode != 0:
head_ref = "main"
+ last_tag = resolve_base_tag(args, head_ref)
+
rng = f"{last_tag}..{head_ref}"Also applies to: 171-184
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 118-122: Command coming from incoming request
Context: subprocess.run(
["git", "rev-parse", "--verify", "--quiet", f"{args.since}^{{commit}}"],
capture_output=True,
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🤖 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 @.agents/skills/release-digest/scripts/gather_changes.py around lines 109 -
137, Update resolve_base_tag to accept a head_ref parameter, handle and validate
args.since before discovering the latest tag, and pass head_ref to all git
describe calls so tag discovery follows the fetched remote reference rather than
local HEAD. In main(), resolve head_ref before evaluating last_tag and pass it
into resolve_base_tag; preserve pre-release behavior by finding the prior tag
relative to the selected head_ref.
2 participants
No description provided.