fix: handle Windows UTF-8 output and MinerU JSON errors - #3
Conversation
|
Note Docstrings generation - SKIPPED |
|
Caution Docstrings generation - FAILED An unexpected error occurred while opening a pull request: Update is not a fast forward - https://docs.github.com/rest/git/refs#update-a-reference |
|
No open human review comments were found in this PR to create a plan for. |
Docstrings generation was requested by @coderabbitai[bot]. * #3 (comment) The following files were modified: * `content-extract/scripts/content_extract.py` * `search-layer/scripts/search.py`
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@content-extract/scripts/content_extract.py`:
- Around line 99-103: The parsed JSON assigned to variable j from
json.loads(p.stdout) may be a non-dict (e.g., list or primitive) so calls to
j.get(...) will raise; update the post-parse logic around json.loads(p.stdout)
and the later usage at the j.get(...) call to guard the type: after
json.loads(p.stdout) validate isinstance(j, dict) (or coerce to an empty dict)
and only call j.get(...) when j is a dict, otherwise handle the non-dict case
(e.g., treat as parse error or extract expected value differently) so the
structured error contract is preserved; references: variable j,
json.loads(p.stdout), and the j.get(...) usage.
- Around line 105-106: The crash-detection logic incorrectly treats exit code 1
as non-crash; update the condition around p.returncode in content_extract.py
(the block that assigns out = _error_output) so it treats any non-zero exit code
as a crash—e.g., replace the current "if p.returncode not in (0, 1):" check with
a strict non-zero check (p.returncode != 0) so exit code 1 is classified as a
crash and routed through _error_output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7ebb8509-1955-4a3c-ba5c-806a2bf6543f
📒 Files selected for processing (2)
content-extract/scripts/content_extract.pysearch-layer/scripts/search.py
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughTwo scripts receive targeted fixes: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
1 participant
Summary
Fixes #2 by addressing two issues reported in Windows environments:
search.pynow configuresstdout/stderrto UTF-8 at startup (best-effort, silent fallback).content_extract.pynow parsesstdoutas JSON first; only marks as "wrapper crashed" when both exit code is non-zero AND stdout is not valid JSON.Changes
search-layer/scripts/search.py: Added_configure_stdio_utf8()helper, called at module load.content-extract/scripts/content_extract.py: Refactored subprocess result handling to distinguish structured JSON errors from true crashes; added_error_output()helper for consistent error payloads.Validation
reconfigure(encoding='utf-8')is called on streams that support it.exit code 2+ valid JSON error payload is correctly classified as structured error (not "wrapper crashed").Notes
itemsis empty, the script now preserves the downstream non-zero return code (e.g.2) instead of always returning1, allowing structured error payloads to propagate their original exit code.Closes #2.
Summary by CodeRabbit
Bug Fixes
Chores