Skip to content

[codex] Fix open contribution issues#292

Merged
vinitkumar merged 5 commits intomasterfrom
codex/fix-open-issues-288-291
May 4, 2026
Merged

[codex] Fix open contribution issues#292
vinitkumar merged 5 commits intomasterfrom
codex/fix-open-issues-288-291

Conversation

@vinitkumar
Copy link
Copy Markdown
Owner

@vinitkumar vinitkumar commented May 4, 2026

Summary

  • improves CLI invalid-input messages for missing files, malformed JSON, empty stdin, and no-input TTY usage
  • adds Rust fast-wrapper fallback tests that run without requiring the optional extension
  • adds realistic README and docs examples for API responses, local files, and stdin pipelines
  • documents benchmark reproduction setup, required tools, script scope, and CLI startup-overhead caveats

Fixes #288
Fixes #289
Fixes #290
Fixes #291

Validation

  • python3 -m pytest
  • lat check

Summary by Sourcery

Improve CLI error messaging, add coverage for Rust fast-wrapper fallback behavior, and expand documentation for benchmarks and usage examples.

Bug Fixes:

  • Clarify CLI error messages for invalid JSON strings, empty stdin, missing files, and malformed JSON files, including actionable guidance on how to supply input.

Enhancements:

  • Refine read_input and stdin handling to distinguish missing files from parse errors and provide source-specific error messages.
  • Document benchmark setup, required tools, and interpretation caveats for mixed library and CLI measurements.
  • Document realistic user flows for API usage, local files, and stdin pipelines, keeping examples aligned across README and docs.

Documentation:

  • Expand benchmark documentation with environment recording, setup instructions, and interpretation guidance for CLI versus library benchmarks.
  • Add LAT behavior and tests documentation for CLI failure messages, user examples, and Rust fast-wrapper behavior.

Tests:

  • Strengthen CLI tests to assert specific failure messages for various invalid input scenarios.
  • Add tests ensuring dicttoxml_fast prefers the Rust backend when supported and falls back to Python for unsupported options, special keys, or missing Rust.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.56%. Comparing base (583d397) to head (b61408d).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #292      +/-   ##
==========================================
+ Coverage   96.07%   98.56%   +2.48%     
==========================================
  Files           6        6              
  Lines         484      487       +3     
==========================================
+ Hits          465      480      +15     
+ Misses         19        7      -12     
Flag Coverage Δ
unittests 98.56% <100.00%> (+2.48%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented May 4, 2026

Reviewer's Guide

Refines CLI input error handling, clarifies benchmark/docs behavior, and adds tests for the Rust fast-wrapper selection and Python fallback paths so contributors can validate behavior without the optional extension.

Sequence diagram for updated CLI input handling and error messages

sequenceDiagram
    actor User
    participant CLI
    participant Argparse
    participant ReadInput as read_input
    participant ReadFromStdin as read_from_stdin
    participant ReadFromString as readfromstring
    participant ReadFromJson as readfromjson
    participant ExitWithError as exit_with_error

    User->>CLI: invoke json2xml CLI
    CLI->>Argparse: parse_args
    Argparse-->>CLI: args
    CLI->>ReadInput: read_input(args)

    alt --string provided
        ReadInput->>ReadFromString: readfromstring(args.string)
        ReadFromString-->>ReadInput: JSONValue
        ReadInput-->>CLI: JSONValue

        else invalid JSON in --string
        ReadFromString--xReadInput: StringReadError
        ReadInput->>ExitWithError: Error Invalid JSON in --string input
        ExitWithError-->>User: detailed guidance message
    end

    alt input_file is -
        ReadInput->>ReadFromStdin: read_from_stdin()
        ReadFromStdin-->>ReadInput: JSONValue
        ReadInput-->>CLI: JSONValue

    else input_file path missing
        ReadInput->>ExitWithError: Error JSON file not found with hint
        ExitWithError-->>User: check path or use - for stdin

    else input_file exists but malformed JSON
        ReadInput->>ReadFromJson: readfromjson(args.input_file)
        ReadFromJson--xReadInput: JSONReadError
        ReadInput->>ExitWithError: Error Could not parse JSON file with hint
        ExitWithError-->>User: check file contains valid JSON
    end

    alt stdin has data
        ReadInput->>ReadFromStdin: read_from_stdin()
        ReadFromStdin->>ReadFromString: readfromstring(json_str)
        ReadFromString-->>ReadFromStdin: JSONValue
        ReadFromStdin-->>ReadInput: JSONValue
        ReadInput-->>CLI: JSONValue

        else stdin empty
        ReadFromStdin->>ExitWithError: Error Empty stdin with guidance
        ExitWithError-->>User: pipe JSON or pass file or --string

        else stdin invalid JSON
        ReadFromStdin--xReadFromString: StringReadError
        ReadFromStdin->>ExitWithError: Error Invalid JSON from stdin with guidance
        ExitWithError-->>User: pipe valid JSON or pass file or --string
    end

    alt stdin is TTY and no other inputs
        ReadInput->>ExitWithError: Error No input provided with guidance
        ExitWithError-->>User: pass file, -, or --string/--url
    end
Loading

Class diagram for dicttoxml_fast Rust wrapper and Python fallback behavior

classDiagram
    class DicttoxmlFastFacade {
        +convert(data, options)
    }

    class RustBackend {
        +convert(data, options)
    }

    class PythonBackend {
        +convert(data, options)
    }

    class FastBackendSelector {
        +has_rust_extension
        +supports_options
        +select_backend(data, options)
    }

    DicttoxmlFastFacade --> FastBackendSelector : uses
    FastBackendSelector --> RustBackend : uses_when_available
    FastBackendSelector --> PythonBackend : fallback

    class DicttoxmlFastTests {
        +test_rust_backend_used_when_available()
        +test_python_fallback_when_rust_missing()
        +test_python_fallback_for_unsupported_options()
        +test_python_fallback_for_special_keys()
    }

    DicttoxmlFastTests --> DicttoxmlFastFacade : verifies_behavior
Loading

File-Level Changes

Change Details Files
Tighten CLI input validation and produce actionable, source-specific error messages for strings, files, stdin, and TTY no-input.
  • Update read_input to distinguish missing files from parse errors and to emit messages that include the file path and suggested next actions.
  • Refine stdin handling in read_from_stdin to treat empty or malformed stdin as distinct errors with explicit recovery guidance.
  • Adjust CLI tests to assert on the new, specific stderr messages for invalid strings, empty stdin, nonexistent files, malformed file JSON, and TTY no-input cases.
json2xml/cli.py
tests/test_cli.py
Document benchmark setup, environment requirements, and interpretation of CLI vs library benchmarks to keep contributor runs comparable.
  • Add instructions for recording machine and tool versions before publishing benchmark results.
  • Describe required tools and environment setup for Python, Rust, Go, and Zig benchmarks, including how to install and verify external CLIs.
  • Clarify benchmark scripts’ behavior, especially multi-Python and CLI subprocess measurements, and explain how to interpret CLI startup overhead in results.
BENCHMARKS.md
lat.md/architecture.md
Clarify LAT docs around user-facing examples and CLI failure-message expectations.
  • Define documentation expectations for user examples, emphasizing realistic API/file/stdin flows and scan-friendly output settings.
  • Describe intended CLI failure-message behavior so tests and contributors align on naming the broken input source and suggesting next actions.
lat.md/behavior.md
lat.md/tests.md
Add tests to cover Rust fast-wrapper dispatch and Python fallback behavior without requiring the Rust extension.
  • Introduce a dedicated test module that forces a fake Rust backend to validate when dicttoxml_fast dispatches to Rust vs Python.
  • Assert that supported option combinations call the Rust backend, while unsupported options and special keys force Python fallback to preserve semantics.
  • Verify behavior when the Rust extension is unavailable so contributors can run tests without installing json2xml_rs.
tests/test_dicttoxml_fast_fallback.py
Update public-facing usage docs (README and Sphinx usage guide) with realistic examples consistent with LAT behavior specs.
  • Align README and usage.rst examples with the documented behavior around API usage, local file conversion, and stdin pipelines.
  • Ensure examples use output options (such as pretty=False) that match the scan-friendly guidance documented in LAT behavior specs.
README.rst
docs/usage.rst

Assessment against linked issues

Issue Objective Addressed Explanation
#288 Provide clear, reproducible benchmark steps in BENCHMARKS.md, including required tools, environment setup, and which benchmark scripts to run for each implementation on a clean checkout.
#288 Document what environment details (machine, OS, Python, and tool availability) should be recorded alongside benchmark results so users can compare runs.
#288 Document caveats around CLI benchmark measurements, especially that Go/Zig CLI results include process startup overhead and how that affects comparison with library calls.
#289 Improve CLI error messages for invalid JSON and missing/no input cases so they are clear, short, actionable, and consistent.
#289 Add or update tests to cover the new CLI behavior for invalid JSON, missing files, empty stdin, and no-input TTY usage.
#289 Keep documentation (README/CLI help/behavior specs) accurate with the new CLI error behaviors.
#290 Add at least two realistic JSON-to-XML examples to the README or docs, covering scenarios like API responses, local JSON files, and stdin pipelines, and showing before/after JSON and XML output.
#290 Ensure the new examples are small/scan-friendly and tied to real converter behavior (tested or generated from existing code paths where practical).
#291 Add focused tests that cover the Rust acceleration path’s automatic fallback behavior (including Rust dispatch when available, Python fallback for unsupported options and special keys, and behavior when the Rust extension is missing).
#291 Ensure the new fallback behavior tests run without requiring contributors to build the optional Rust extension.
#291 Clarify and document which options/features are Rust-backed vs pure Python, keeping tests/docs (including README/CONTRIBUTING/docs/lat specs) accurate regarding fallback behavior.

Possibly linked issues

  • #unknown: PR implements clearer CLI errors for invalid JSON and missing input, adds corresponding tests, and updates docs as requested.
  • #: They match: PR adds Rust fast-wrapper fallback tests and docs that cover automatic Rust→Python fallback behavior.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@vinitkumar vinitkumar marked this pull request as ready for review May 4, 2026 07:42
@vinitkumar vinitkumar merged commit b055343 into master May 4, 2026
48 checks passed
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

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

Labels

None yet

Projects

None yet

1 participant