Skip to content

--reanalyze-and-wait: progress line for non-TTY callers - #42

Open
andrzej-janczak wants to merge 1 commit into
mainfrom
feat/reanalyze-wait-progress
Open

--reanalyze-and-wait: progress line for non-TTY callers#42
andrzej-janczak wants to merge 1 commit into
mainfrom
feat/reanalyze-wait-progress

Conversation

@andrzej-janczak

Copy link
Copy Markdown

Problem: --reanalyze-and-wait shows only a spinner. When stderr is not a TTY the wait is silent for up to 20 min, and shell tools that kill silent commands (Gemini CLI, 5 min) abort it; the autoconfig agent then re-imports and re-triggers reanalysis 2-3x per run.
Fix: when stderr is not a TTY, print elapsed Nm, status=<status> to stderr every 60 s. TTY behavior, 10 s poll and 20 min cap unchanged.

ora's spinner text never reaches a piped stderr, so a silent
--reanalyze-and-wait wait could look hung to CI/agent shells.
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 16 complexity · 8 duplication

Metric Results
Complexity 16
Duplication 8

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

@codacy-production codacy-production Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

The PR successfully implements periodic progress logging to stderr for non-TTY callers, addressing potential CI/agent timeout issues during long-running analysis waits. The implementation correctly maintains existing TTY behavior and adheres to the required polling intervals and phases.

Codacy analysis indicates the changes are up to standards. Recommendations are provided to stabilize the internal heartbeat clock and deduplicate identical polling logic used across the 'waiting' and 'inProgress' phases to improve long-term maintainability.

1 comment outside of the diff
src/utils/reanalyze-wait.ts

line 329-345 ⚪ LOW RISK
Suggestion: The polling loop bodies in Phase A and Phase B are identical. Extracting this common logic into a helper function would improve maintainability and ensure polling behavior remains consistent across both phases.

Refactor the pollForAnalysis function to deduplicate the polling loop body by creating a helper function for the common steps (timeout check, sleep, status fetch, and progress logging) while keeping the two-phase logic clear.

Test suggestions

  • Logs 'waiting' status during the initial phase when stderr is not a TTY
  • Logs 'inProgress' status during the second phase when stderr is not a TTY
  • Does not print progress lines when stderr is a TTY
  • Observes the 60-second interval (does not print before 60s elapses)
  • Correctly increments the elapsed minute count in sequential logs

TIP How was this review? Give us feedback

Comment on lines +317 to +323
const maybeLogProgress = () => {
if (isTTY) return;
while (now() - startedAt >= nextProgressAtMs) {
process.stderr.write(`elapsed ${nextProgressAtMs / 60_000}m, status=${phase}\n`);
nextProgressAtMs += PROGRESS_INTERVAL_MS;
}
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚪ LOW RISK

Suggestion: Capture the current timestamp once at the beginning of the function and ensure the minute calculation result is an integer if the interval is changed in the future.

Suggested change
const maybeLogProgress = () => {
if (isTTY) return;
while (now() - startedAt >= nextProgressAtMs) {
process.stderr.write(`elapsed ${nextProgressAtMs / 60_000}m, status=${phase}\n`);
nextProgressAtMs += PROGRESS_INTERVAL_MS;
}
};
const maybeLogProgress = () => {
if (isTTY) return;
const currentNow = now();
while (currentNow - startedAt >= nextProgressAtMs) {
process.stderr.write(`elapsed ${Math.floor(nextProgressAtMs / 60_000)}m, status=${phase}\n`);
nextProgressAtMs += PROGRESS_INTERVAL_MS;
}
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant