Skip to content

Conversation

matdev83
Copy link
Owner

@matdev83 matdev83 commented Oct 12, 2025

Summary

  • ensure the streaming JSON repair processor re-raises existing JSONParsingError and ValidationError instances in strict mode instead of wrapping them
  • add a regression test covering ValidationError propagation for strict streaming repair

Testing

  • python -m pytest --override-ini=addopts='' tests/unit/json_repair_processor_test.py
  • python -m pytest --override-ini=addopts=''

https://chatgpt.com/codex/tasks/task_e_68ec26ffb7448333ace9a74c4ba180a1

Summary by CodeRabbit

  • Bug Fixes

    • Improved strict mode behavior in streaming JSON processing: validation-related failures are now surfaced directly to callers, aligning with parsing error handling and providing clearer, more actionable error feedback.
  • Tests

    • Added unit tests to verify that strict mode correctly propagates validation failures during JSON repair and validation.
Copy link

coderabbitai bot commented Oct 12, 2025

Walkthrough

Updated strict-mode error handling in JsonRepairProcessor to re-raise ValidationError alongside JSONParsingError. Added a unit test with a raising validation service to assert propagation of ValidationError in strict mode.

Changes

Cohort / File(s) Summary
Processor error handling
src/core/services/streaming/json_repair_processor.py
Import ValidationError; in strict mode, re-raise ValidationError similarly to JSONParsingError; otherwise wrap in JSONParsingError with structured message; logging/metrics behavior unchanged.
Unit tests for strict-mode propagation
tests/unit/json_repair_processor_test.py
Add RaisingValidationService test double that raises ValidationError; add test asserting ValidationError propagates in strict mode; import pytest and ValidationError.

Sequence Diagram(s)

sequenceDiagram autonumber actor Caller participant Processor as JsonRepairProcessor participant Service as JsonRepairService Caller->>Processor: process_chunk(json_chunk, strict_mode) Processor->>Service: repair_and_validate_json(...) alt Success Service-->>Processor: repaired_json Processor-->>Caller: result else ValidationError Service--x Processor: ValidationError alt strict_mode = True note right of Processor: Re-raise ValidationError (no wrapping) Processor--x Caller: ValidationError else strict_mode = False note right of Processor: Wrap as JSONParsingError Processor--x Caller: JSONParsingError end else Other Exception Service--x Processor: Exception note right of Processor: Log, wrap as JSONParsingError Processor--x Caller: JSONParsingError end 
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A rabbit taps the JSON stream,
Hops through braces, swift and keen.
In strict-mode fields where errors glare,
Validation squeaks? We leave it bare.
No wrapping cloak, the truth laid clear—
Thump! The tests now cheer. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately captures the core update of the pull request by stating that streaming JSON repair will now propagate validation errors, directly reflecting the changes to strict‐mode behavior without extraneous detail.
✨ 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 codex/fix-identified-bug-in-files-i-k

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dd216cf and 014059e.

📒 Files selected for processing (2)
  • src/core/services/streaming/json_repair_processor.py (2 hunks)
  • tests/unit/json_repair_processor_test.py (3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Avoid using emojis in Python code comments or docstrings
Follow PEP 8 and use type hints for all functions
Import order must be: standard library, third-party, then local imports, separated by blank lines
Naming conventions: snake_case for variables and functions; PascalCase for classes
Error handling: use specific exceptions and include meaningful error messages
Prefer f-strings for string formatting

Files:

  • src/core/services/streaming/json_repair_processor.py
  • tests/unit/json_repair_processor_test.py
src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Catch specific exceptions; avoid broad except Exception blocks
If a broad exception must be caught, log with exc_info=True and re-raise a specific custom exception
Use the most specific exception class from src.core.common.exceptions that accurately describes the error
Provide clear, helpful error messages and include relevant details in the details dictionary of custom exceptions

Files:

  • src/core/services/streaming/json_repair_processor.py
🧬 Code graph analysis (2)
src/core/services/streaming/json_repair_processor.py (1)
src/core/common/exceptions.py (2)
  • JSONParsingError (231-238)
  • ValidationError (123-129)
tests/unit/json_repair_processor_test.py (3)
src/core/services/json_repair_service.py (1)
  • JsonRepairService (16-292)
src/core/services/streaming/json_repair_processor.py (2)
  • JsonRepairProcessor (18-221)
  • process (49-103)
src/core/domain/streaming_content.py (1)
  • StreamingContent (10-158)
🪛 Ruff (0.13.3)
tests/unit/json_repair_processor_test.py

30-30: Unused method argument: json_string

(ARG002)


31-31: Unused method argument: schema

(ARG002)


32-32: Unused method argument: strict

(ARG002)

⏰ 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). (2)
  • GitHub Check: test (3.10)
  • GitHub Check: Analyze (python)
🔇 Additional comments (5)
src/core/services/streaming/json_repair_processor.py (2)

8-8: LGTM!

The import of ValidationError is necessary for the strict-mode exception handling below.


167-168: LGTM!

The updated exception handling correctly re-raises both JSONParsingError and ValidationError in strict mode, ensuring these domain-specific exceptions propagate to the caller for proper handling. This aligns with the PR objectives and maintains consistency with how JSONParsingError was previously handled.

tests/unit/json_repair_processor_test.py (3)

6-7: LGTM!

The imports are necessary for the new test that validates ValidationError propagation.


25-35: LGTM!

The test double correctly raises ValidationError to simulate validation failures during repair. The unused parameter warnings from static analysis are false positives—test doubles intentionally override method signatures without using all parameters.


65-75: LGTM!

The test correctly validates that ValidationError is propagated when strict_mode=True, addressing the regression described in the PR objectives.


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

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

Labels

1 participant