UN-2175 Avoid extracting when doc is already extracted during indexing #1605

chandrasekharan-zipstack · 2025-10-22T16:22:09Z

What

Removed reindex parameter from dynamic_extractor()
Removed a check involving the reindex parameter

Why

Once a document is extracted and the text is available, we needn't re-extract it

How

Removed redundant check of reindex during text extraction

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

No, the reindex parameter was only used to determine if a document should be extracted and was always enforced. Now we check and extract only if it required

Related Issues or PRs

Noticed while working with UN-2175

Notes on Testing

Tested via the explicit index and it worked as expected

Checklist

I have read and understood the Contribution Guidelines.

coderabbitai · 2025-10-22T16:22:37Z

Summary by CodeRabbit

Refactor
- Simplified document extraction logic by removing the reindex option. Previously extracted document content will now be consistently reused rather than re-processed.

Walkthrough

The reindex parameter was removed from the dynamic_extractor method signature in the PromptStudioHelper class. The method call in index_document was updated to reflect this removal, and the conditional logic for determining whether to return previously extracted content was simplified.

Changes

Cohort / File(s)	Summary
Parameter removal and control flow simplification `backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py`	Removed `reindex` parameter from `dynamic_extractor` method signature; updated corresponding call in `index_document` to remove the `reindex=True` argument; simplified conditional logic from checking `is_extracted and not reindex` to checking only `is_extracted`

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title "UN-2175 Avoid extracting when doc is already extracted during indexing" is directly related to the main change in the changeset. It accurately summarizes the primary modification: removing the reindex parameter and simplifying the extraction logic to skip unnecessary re-extraction of already-extracted documents. The title is concise, specific, and free of vague or unnecessary elements, clearly conveying the purpose of the change to someone reviewing the commit history.
Description Check	✅ Passed	The pull request description follows the required template structure and includes all critical sections. The "What" section explains the removal of the reindex parameter and related checks, the "Why" section provides clear rationale about avoiding unnecessary re-extraction, and the "How" section describes the implementation. Most importantly, the critical section "Can this PR break any existing features" is thoroughly completed with an explanation of why no breakage is expected. The description also includes the "Related Issues or PRs" reference to UN-2175, "Notes on Testing" confirming explicit index testing, and the contributor checklist confirmation, demonstrating completeness and adherence to the template.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch UN-2175-avoid-extracting-when-already-extracted

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to Reviews > Disable Cache setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 91f17dc and f748599.

📒 Files selected for processing (1)

backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: build

🔇 Additional comments (3)

backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py (3)

351-351: LGTM: Simplified logic aligns with PR objectives.

The conditional now correctly returns cached extraction whenever the document is already extracted. This eliminates unnecessary re-extraction and aligns with the stated goal that "if a document is already extracted and text is available, re-extraction is unnecessary."

Note: The fallback error handling at lines 1360-1365 ensures that if the extracted file is missing despite is_extracted being True, extraction will proceed, providing good resilience.

422-430: Call site correctly updated.

The removal of the reindex=True argument is consistent with the updated dynamic_extractor signature.

1326-1334: All call sites correctly updated; no external callers found.

Verification confirms the reindex parameter removal is safely applied throughout the codebase. All three internal call sites within the file have been updated correctly (lines 422, 880, 1250). No external calls to dynamic_extractor exist outside this file—the only imports of PromptStudioHelper in other modules do not invoke this method. The conditional logic change at line 1351 (simplified to if is_extracted:) correctly reflects the intended behavior. The test reference to reindex in test_api_endpoints.py is unrelated, occurring in a different service's API layer.

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.}

Deepak-Kesavan

Looks good.

github-actions · 2025-10-27T05:24:30Z

Test Results

Summary

✅ Runner Tests: 11 passed, 0 failed (11 total)
✅ SDK1 Tests: 66 passed, 0 failed (66 total)

Runner Tests - Full Report

filepath	function	$$\textcolor{#23d18b}{\tt{passed}}$$	SUBTOTAL
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$	$$\textcolor{#23d18b}{\tt{test\_logs}}$$	$$\textcolor{#23d18b}{\tt{1}}$$	$$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$	$$\textcolor{#23d18b}{\tt{test\_cleanup}}$$	$$\textcolor{#23d18b}{\tt{1}}$$	$$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$	$$\textcolor{#23d18b}{\tt{test\_cleanup\_skip}}$$	$$\textcolor{#23d18b}{\tt{1}}$$	$$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$	$$\textcolor{#23d18b}{\tt{test\_client\_init}}$$	$$\textcolor{#23d18b}{\tt{1}}$$	$$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$	$$\textcolor{#23d18b}{\tt{test\_get\_image\_exists}}$$	$$\textcolor{#23d18b}{\tt{1}}$$	$$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$	$$\textcolor{#23d18b}{\tt{test\_get\_image}}$$	$$\textcolor{#23d18b}{\tt{1}}$$	$$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$	$$\textcolor{#23d18b}{\tt{test\_get\_container\_run\_config}}$$	$$\textcolor{#23d18b}{\tt{1}}$$	$$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$	$$\textcolor{#23d18b}{\tt{test\_get\_container\_run\_config\_without\_mount}}$$	$$\textcolor{#23d18b}{\tt{1}}$$	$$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$	$$\textcolor{#23d18b}{\tt{test\_run\_container}}$$	$$\textcolor{#23d18b}{\tt{1}}$$	$$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$	$$\textcolor{#23d18b}{\tt{test\_get\_image\_for\_sidecar}}$$	$$\textcolor{#23d18b}{\tt{1}}$$	$$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$	$$\textcolor{#23d18b}{\tt{test\_sidecar\_container}}$$	$$\textcolor{#23d18b}{\tt{1}}$$	$$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{TOTAL}}$$		$$\textcolor{#23d18b}{\tt{11}}$$	$$\textcolor{#23d18b}{\tt{11}}$$

SDK1 Tests - Full Report

sonarqubecloud · 2025-10-27T05:24:40Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

#1605) Co-authored-by: harini-venkataraman <115449948+harini-venkataraman@users.noreply.github.com>

UN-2175 Avoid extracting when doc is already extracted during indexing

f748599

chandrasekharan-zipstack requested review from Deepak-Kesavan and harini-venkataraman October 22, 2025 16:22

chandrasekharan-zipstack self-assigned this Oct 22, 2025

harini-venkataraman approved these changes Oct 23, 2025

View reviewed changes

Merge branch 'main' into UN-2175-avoid-extracting-when-already-extracted

b43d04f

Deepak-Kesavan reviewed Oct 24, 2025

View reviewed changes

Deepak-Kesavan approved these changes Oct 27, 2025

View reviewed changes

Merge branch 'main' into UN-2175-avoid-extracting-when-already-extracted

09d6091

chandrasekharan-zipstack merged commit 13642ba into main Oct 27, 2025
7 checks passed

chandrasekharan-zipstack deleted the UN-2175-avoid-extracting-when-already-extracted branch October 27, 2025 05:30

muhammad-ali-e pushed a commit that referenced this pull request Oct 30, 2025

UN-2175 Avoid extracting when doc is already extracted during indexing (

fbad0d3

#1605) Co-authored-by: harini-venkataraman <115449948+harini-venkataraman@users.noreply.github.com>

muhammad-ali-e pushed a commit that referenced this pull request Oct 30, 2025

UN-2175 Avoid extracting when doc is already extracted during indexing (

1e96236

#1605) Co-authored-by: harini-venkataraman <115449948+harini-venkataraman@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

UN-2175 Avoid extracting when doc is already extracted during indexing #1605

UN-2175 Avoid extracting when doc is already extracted during indexing #1605

Uh oh!

chandrasekharan-zipstack commented Oct 22, 2025 •

edited by atlassian bot

Loading

Uh oh!

coderabbitai bot commented Oct 22, 2025 •

edited

Loading

Uh oh!

Deepak-Kesavan left a comment

Uh oh!

github-actions bot commented Oct 27, 2025

Uh oh!

sonarqubecloud bot commented Oct 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

UN-2175 Avoid extracting when doc is already extracted during indexing #1605

UN-2175 Avoid extracting when doc is already extracted during indexing #1605

Uh oh!

Conversation

chandrasekharan-zipstack commented Oct 22, 2025 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

How

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

Related Issues or PRs

Notes on Testing

Checklist

Uh oh!

coderabbitai bot commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

Deepak-Kesavan left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 27, 2025

Test Results

Uh oh!

sonarqubecloud bot commented Oct 27, 2025

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chandrasekharan-zipstack commented Oct 22, 2025 •

edited by atlassian bot

Loading

coderabbitai bot commented Oct 22, 2025 •

edited

Loading