feat: make resume baseline explicit from SDLC ledger - #58
Conversation
📝 WalkthroughWalkthroughThis change adds a baseline resume log line that outputs the run_id and state before processing the SDLC ledger summary during resume operations. The modification alters control flow structure with explicit braces but preserves existing error handling behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
|
Addressed current actionable CI blocker:\n- ran cargo fmt and pushed c24ed55 to fix the failing check.\n\nValidation rerun is now in progress. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/cli/run.rs (2)
101-103: Warning message could include actionable guidance.Per coding guidelines, user-facing errors in
src/cli/**/*.rsshould include actionable guidance. The current warning doesn't tell operators what to do next.💡 Suggested improvement
Err(err) => { - eprintln!("warn: failed to format SDLC run ledger summary: {err}"); + eprintln!("warn: failed to format SDLC run ledger summary: {err}; resuming without summary"); }As per coding guidelines: "User-facing errors should include actionable guidance".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/run.rs` around lines 101 - 103, The warning logged in the Err arm that currently prints "warn: failed to format SDLC run ledger summary: {err}" should be expanded to provide actionable guidance to operators: update the `Err(err)` branch in src/cli/run.rs (the SDLC run ledger summary formatting code path) to include next steps such as verifying the ledger input/format, retrying the command, and where to find logs or how to report the error (e.g., “check ledger file/permissions, rerun with --verbose, and if the issue persists open an issue with the following error”). Keep the original error `{err}` in the message for diagnostics and make the guidance concise and specific.
94-105: Consider deduplicating run_id/state output or clarifying intent.The new baseline log (lines 95-98) outputs
run_idandstate, thensdlc_pr_comment_summaryimmediately outputs these same values again (persrc/state/mod.rs:215-218:"SDLC run '{}' for #{} is currently '{:?}'..."). This produces redundant output like:Resume baseline from SDLC ledger: run='abc123' state='Branched' SDLC run `abc123` for `#42` is currently `Branched` (updated ...)If the duplication is intentional for clarity in recovery flows, consider adding a brief comment explaining why both lines are needed. Otherwise, you could remove the duplicate fields from the baseline or adjust
sdlc_pr_comment_summaryto skip them when called during resume.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/run.rs` around lines 94 - 105, The resume baseline message in run.rs currently prints run_id/state and then calls sdlc_pr_comment_summary which prints the same fields again; to fix, remove the duplicate fields from the eprintln in the Ok(Some(ledger)) arm (keep a concise "Resume baseline from SDLC ledger" message or nothing) and rely on sdlc_pr_comment_summary(&ledger) to emit the full, formatted summary, or alternatively modify sdlc_pr_comment_summary in src/state/mod.rs to accept a flag like include_header: bool and pass false from run.rs; update call sites accordingly so the run_id/state are only printed once (adjust the Ok(Some(ledger)) eprintln or the sdlc_pr_comment_summary signature as chosen).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/cli/run.rs`:
- Around line 101-103: The warning logged in the Err arm that currently prints
"warn: failed to format SDLC run ledger summary: {err}" should be expanded to
provide actionable guidance to operators: update the `Err(err)` branch in
src/cli/run.rs (the SDLC run ledger summary formatting code path) to include
next steps such as verifying the ledger input/format, retrying the command, and
where to find logs or how to report the error (e.g., “check ledger
file/permissions, rerun with --verbose, and if the issue persists open an issue
with the following error”). Keep the original error `{err}` in the message for
diagnostics and make the guidance concise and specific.
- Around line 94-105: The resume baseline message in run.rs currently prints
run_id/state and then calls sdlc_pr_comment_summary which prints the same fields
again; to fix, remove the duplicate fields from the eprintln in the
Ok(Some(ledger)) arm (keep a concise "Resume baseline from SDLC ledger" message
or nothing) and rely on sdlc_pr_comment_summary(&ledger) to emit the full,
formatted summary, or alternatively modify sdlc_pr_comment_summary in
src/state/mod.rs to accept a flag like include_header: bool and pass false from
run.rs; update call sites accordingly so the run_id/state are only printed once
(adjust the Ok(Some(ledger)) eprintln or the sdlc_pr_comment_summary signature
as chosen).
2 participants
Summary\n- print an explicit baseline line before SDLC ledger summary during \n- surface the exact run id + SDLC state being resumed from to reduce ambiguity in recovery flows\n\n## Validation\n- cargo test -q sdlc_pr_comment_summary_renders_transitions\n- cargo test -q serialize_dry_run_contains_prompt_and_command_steps\n\nFixes #30
Summary by CodeRabbit