Skip to content

[Follow-up] In tests/test_structured_output.py, modify the par (PR #1372) #1395

Description

@stranske

Why

PR #1372 addressed issue #1371, and verification passed, but it surfaced a few test-structure and environment gaps. This follow-up tightens test intent by validating production-level wiring (not internal helpers), makes expected structured-output behavior explicit in parametrized tests, strengthens fallback provider assertions (including identity forwarding of quality_context and ensuring only the selected provider is called), and fixes dependency pins so a fresh install can run the full test suite reliably.

Source

Tasks

  • Update tests/test_structured_output.py to spy/mock at the production-level repair-loop callsite (the externally visible API path that triggers repairs) to capture the effective max_repair_attempts argument, avoiding any monkeypatch/spy of internal helpers like _invoke_repair_loop.
  • Modify the structured output parameterized test to include an explicit expected_effective parameter in the param list for max_repair_attempts values [0, 1, 2, 10] with mapping [0, 1, 1, 1], and add an inline comment adjacent to the assertion explaining the production rule (e.g., clamping to [0, 1]).
  • Strengthen tests/test_fallback_chain_provider.py to (a) pass a unique sentinel as quality_context to analyze_completion, (b) assert the selected provider is called exactly once and call_args.kwargs["quality_context"] is sentinel (identity), and (c) assert all other providers have call_count == 0.
  • Adjust requirements.txt to pin pandas to an exact version that is known to be available on PyPI (replace pandas==3.0.0 if necessary) while keeping exact == pins, and add any missing exact-version dependencies required to import and run the full test suite in a fresh environment (e.g., pytest and other test-imported modules).

Acceptance Criteria

  • tests/test_structured_output.py includes exactly one @pytest.mark.parametrize for max_repair_attempts that enumerates the four input values [0, 1, 2, 10] and also parameterizes an explicit expected_effective value for each case with the stated mapping [(0,1), (1,1), (2,1), (10,1)] or equivalent row-wise representation.
  • The structured-output test captures the effective max_repair_attempts by spying/mocking the production-level repair-loop callsite that is invoked via the externally visible structured-output API path, and does not spy/mock any internal helper functions (e.g., no monkeypatch/spy of _invoke_repair_loop).
  • For each structured-output parametrized case, the test asserts that the captured argument passed into the repair-loop callsite equals the per-row expected_effective value, and the assertion line has an adjacent inline comment that states the production rule (e.g., that max_repair_attempts is clamped to a defined range such as [0, 1]).
  • tests/test_fallback_chain_provider.py passes a unique sentinel object as quality_context to FallbackChainProvider.analyze_completion and asserts the selected provider mock is called exactly once.
  • tests/test_fallback_chain_provider.py asserts the selected provider was called with call_args.kwargs["quality_context"] that is the exact same object instance as the sentinel (identity check using is, not equality).
  • tests/test_fallback_chain_provider.py asserts that every non-selected provider mock in the chain has call_count == 0 after analyze_completion completes.
  • requirements.txt does not contain pandas==3.0.0 and instead pins pandas to a real, currently published PyPI version using an exact == pin.
  • Every dependency listed in requirements.txt uses exact version pins (must match the pattern package_name==version with no unpinned entries, no ranges, and no -e).
  • Creating a fresh virtual environment, installing from requirements.txt, and running the full test suite completes without ModuleNotFoundError.

Implementation Notes

  • Structured-output test spying: Patch/spy the public/production callsite that the structured-output API uses to trigger the repair loop, and capture the max_repair_attempts argument at that boundary. Avoid any spying/patching of internal helper functions (especially _invoke_repair_loop), even if they’re currently convenient.
  • Parametrization: Use a single parametrized test for the four max_repair_attempts inputs, and add an expected_effective column per row. Keep the assertion close to where the captured argument is checked, and place the inline clamp-rule comment immediately adjacent to the assertion.
  • Fallback chain provider test:
    • Create a sentinel via sentinel = object().
    • Call analyze_completion(..., quality_context=sentinel).
    • Assert the selected provider mock call_count == 1.
    • Assert selected_provider.call_args.kwargs["quality_context"] is sentinel.
    • Assert each other provider mock in the chain has call_count == 0 (explicit assertions per provider).
  • Requirements:
    • Replace pandas==3.0.0 with a known-published version pinned with ==.
    • Ensure all entries are exact pins (==) and include any test-import dependencies needed to run the full suite in a fresh environment.
Background (previous attempt context)
  • Failure to avoid: Relying on a monkeypatched internal helper (_invoke_repair_loop) to capture repair attempt counts.
    Why it failed: This does not verify production callsite behavior.
    Do instead: Implement a spy or mock at the production-level API path that actually triggers the repair loop, ensuring captured arguments represent real execution wiring.

Critical Rules

  1. Do NOT include "Remaining Unchecked Items" or "Iteration Details" sections unless they contain specific, useful failure context
  2. Tasks should be concrete actions, not verification concerns restated
  3. Acceptance criteria must be testable (not "all concerns addressed")
  4. Keep the main body focused - hide background/history in the collapsible section
  5. Do NOT include the entire analysis object - only include specific failure contexts from blockers_to_avoid

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions