Skip to content

feat(parsing): advance to next phase when status is inferring#384

Open
shmbhvi101 wants to merge 2 commits into
mainfrom
fix/inf-status
Open

feat(parsing): advance to next phase when status is inferring#384
shmbhvi101 wants to merge 2 commits into
mainfrom
fix/inf-status

Conversation

@shmbhvi101

@shmbhvi101 shmbhvi101 commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Advance to next phase when status is either "inferring" or "Ready"

Summary by CodeRabbit

  • Refactor
    • Improved logic for advancing chat steps to make step progression more reliable.
    • Enhanced parsing status handling so the UI updates more consistently and returns promptly when parsing is complete.

@shmbhvi101 shmbhvi101 requested a review from nndn April 13, 2026 09:24
@coderabbitai

coderabbitai Bot commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Introduces a shouldAdvanceToNextStep(status) helper and refactors pollParsingStatus in services/BranchAndRepositoryService.ts to perform an early initial-status check, update chatStep and setParsingStatus accordingly, and use the helper inside the polling loop to decide when to advance or return.

Changes

Cohort / File(s) Summary
Parsing status polling
services/BranchAndRepositoryService.ts
Added shouldAdvanceToNextStep(status) helper; added pre-loop initial-status handling that can set chatStep and setParsingStatus and return early if status is READY; replaced in-loop parsingStatus === READY logic with shouldAdvanceToNextStep(parsingStatus) and adjusted mapping to preserve READY or convert other qualifying statuses via getStatusMessage.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • yashkrishan

Poem

🐰 In the meadow of code I hop and peep,
A helper made so checks don't creep,
INFERRING and READY line the lane,
Polling flows tidy, no more strain,
Hooray — a cleaner hop, and off I leap! 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: introducing logic to advance to the next phase when parsing status is 'inferring'.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/inf-status

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@services/BranchAndRepositoryService.ts`:
- Around line 343-352: The current early return when
shouldAdvanceToNextStep(parsingStatus) is true causes the poller to stop on
INFERRING; change the logic in the block that calls setChatStep and
setParsingStatus so it only returns/terminates when parsingStatus ===
ParsingStatusEnum.READY (i.e., allow INFERRING to continue polling), and update
the other analogous block (the one around the later return) the same way;
reference setChatStep, setParsingStatus, shouldAdvanceToNextStep, and
ParsingStatusEnum so you only short-circuit for READY and let pollParsingStatus
continue monitoring for later READY/ERROR transitions.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b8cf2490-9c98-4fdd-9c42-cc18782d8fad

📥 Commits

Reviewing files that changed from the base of the PR and between c2db84c and 5a6d82e.

📒 Files selected for processing (1)
  • services/BranchAndRepositoryService.ts

Comment thread services/BranchAndRepositoryService.ts Outdated
@github-actions

Copy link
Copy Markdown

Cloud Run service deployed: https://pr-fix-inf-status-cj6r7x3fpa-uc.a.run.app

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
services/BranchAndRepositoryService.ts (1)

343-355: Extract the duplicated transition block.

The same setChatStep / setParsingStatus sequence now exists in two places. Pulling it into one helper will keep the pre-loop and in-loop paths from drifting again.

♻️ Suggested cleanup
         const shouldAdvanceToNextStep = (status: string) =>
           status === ParsingStatusEnum.INFERRING ||
           status === ParsingStatusEnum.READY;
+
+        const applyTransition = (status: string) => {
+          if (!shouldAdvanceToNextStep(status)) return false;
+
+          setChatStep?.(2);
+          setParsingStatus(
+            status === ParsingStatusEnum.READY
+              ? ParsingStatusEnum.READY
+              : getStatusMessage(status)
+          );
+
+          return status === ParsingStatusEnum.READY;
+        };
 
-        if (shouldAdvanceToNextStep(parsingStatus)) {
-          if (setChatStep) {
-            setChatStep(2);
-          }
-          setParsingStatus(
-            parsingStatus === ParsingStatusEnum.READY
-              ? ParsingStatusEnum.READY
-              : getStatusMessage(parsingStatus)
-          );
-          if (parsingStatus === ParsingStatusEnum.READY) {
-            return;
-          }
-        }
+        if (applyTransition(parsingStatus)) return;
@@
-          if (shouldAdvanceToNextStep(parsingStatus)) {
-            if (setChatStep) {
-              setChatStep(2); 
-            }
-            setParsingStatus(
-              parsingStatus === ParsingStatusEnum.READY
-                ? ParsingStatusEnum.READY
-                : getStatusMessage(parsingStatus)
-            );
-            if (parsingStatus === ParsingStatusEnum.READY) {
-              return;
-            }
-          }
+          if (applyTransition(parsingStatus)) return;

Also applies to: 361-372

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/BranchAndRepositoryService.ts` around lines 343 - 355, There is a
duplicated transition block that updates chat step and parsing status in two
places; create a helper function (e.g., applyParsingTransition or
updateChatAndParsingStatus) that accepts parsingStatus and
setChatStep/setParsingStatus callbacks and encapsulates the logic currently in
the block (calls setChatStep(2) if provided, then sets parsing status to READY
or getStatusMessage(parsingStatus), and returns early if parsingStatus ===
ParsingStatusEnum.READY). Replace the two occurrences (the block inside
shouldAdvanceToNextStep handling and the similar block at lines ~361-372) with
calls to this helper to avoid drift and keep behavior centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@services/BranchAndRepositoryService.ts`:
- Around line 343-355: There is a duplicated transition block that updates chat
step and parsing status in two places; create a helper function (e.g.,
applyParsingTransition or updateChatAndParsingStatus) that accepts parsingStatus
and setChatStep/setParsingStatus callbacks and encapsulates the logic currently
in the block (calls setChatStep(2) if provided, then sets parsing status to
READY or getStatusMessage(parsingStatus), and returns early if parsingStatus ===
ParsingStatusEnum.READY). Replace the two occurrences (the block inside
shouldAdvanceToNextStep handling and the similar block at lines ~361-372) with
calls to this helper to avoid drift and keep behavior centralized.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8694164f-dabe-48a6-b523-32d7042ccd87

📥 Commits

Reviewing files that changed from the base of the PR and between 5a6d82e and cc2a8b6.

📒 Files selected for processing (1)
  • services/BranchAndRepositoryService.ts

@github-actions

Copy link
Copy Markdown

Cloud Run service deployed: https://pr-fix-inf-status-cj6r7x3fpa-uc.a.run.app

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