Skip to content

test: stabilize profile workflow e2e fixtures - #366

Merged
guangyu-reflexio merged 1 commit into
mainfrom
fix/e2e-profile-workflow-fixtures
Jul 18, 2026
Merged

test: stabilize profile workflow e2e fixtures#366
guangyu-reflexio merged 1 commit into
mainfrom
fix/e2e-profile-workflow-fixtures

Conversation

@guangyu-reflexio

@guangyu-reflexio guangyu-reflexio commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Stabilizes the direct-library profile workflow E2E tests by removing live should-run classifier variance from fixtures that are testing profile storage/search/rerun mechanics.
  • Keeps the customer-support scenario faithful by widening the extraction window and aligning the profile definition with the scenario's durable customer facts.
  • Hardens structured-output parsing so single-field optional list schemas, such as profile extraction output, accept provider responses that arrive as a top-level list.

Changes

  • Added optional-list detection to the structured-output single-list wrapper path.
  • Added unit coverage for top-level lists against list[...] | None wrapper schemas.
  • Updated E2E profile fixtures to bypass the should-run gate and include the full customer-support scenario window.

Test Plan

  • uv run --no-sync ruff check open_source/reflexio/reflexio/server/llm/_litellm_structured_output.py open_source/reflexio/tests/e2e_tests/conftest.py open_source/reflexio/tests/server/llm/test_litellm_client_unit.py
  • uv run --no-sync pyright open_source/reflexio/reflexio/server/llm/_litellm_structured_output.py open_source/reflexio/tests/e2e_tests/conftest.py open_source/reflexio/tests/server/llm/test_litellm_client_unit.py
  • uv run --no-sync pytest open_source/reflexio/tests/server/llm/test_litellm_client_unit.py -q -o addopts=
  • uv run --no-sync pytest open_source/reflexio/tests/e2e_tests/test_profile_workflows.py -m e2e -o addopts=
  • uv run --no-sync pytest open_source/reflexio/tests/e2e_tests/test_interaction_workflows.py -m e2e -o addopts=

Summary by CodeRabbit

  • Bug Fixes

    • Improved structured response handling for list data wrapped in optional or union-based schemas.
    • Top-level list responses are now correctly parsed into compatible response models.
    • Preserved validation for schemas with multiple fields that cannot safely wrap list responses.
  • Tests

    • Added coverage for optional list fields and customer-support profile extraction scenarios.

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The structured-output parser now recognizes optional or union-wrapped list fields when wrapping top-level JSON arrays. Unit tests cover optional, bare, and multi-field schemas. E2e fixtures share customer-support extraction settings and skip the run check.

Changes

Structured output parsing

Layer / File(s) Summary
Nested list-schema detection and validation
reflexio/server/llm/_litellm_structured_output.py, tests/server/llm/test_litellm_client_unit.py
List annotations are detected recursively through unions and generics, with tests covering optional-list wrapping, bare-list wrapping, and rejected multi-field schemas.

Customer-support e2e fixtures

Layer / File(s) Summary
Shared fixture configuration
tests/e2e_tests/conftest.py
Both fixtures use shared window-size and extraction-prompt constants and set skip_should_run_check=True.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: yyiilluu, yilu331

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: stabilizing profile workflow e2e fixtures.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/e2e-profile-workflow-fixtures

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
reflexio/server/llm/_litellm_structured_output.py (1)

56-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Restrict recursive list detection to Union and Annotated types.

The current implementation recurses into the generic arguments of any type, including dict, tuple, and Callable. While Pydantic's downstream validation mitigates practical risks by failing on invalid types anyway, the logic conceptually violates its intent ("Return whether an annotation accepts a list value") by returning True for types like dict[str, list[int]].

Consider restricting the recursion to Union (which handles Optional and Python 3.10 | unions) and Annotated types to ensure the list detection remains precise.

💡 Proposed refactor to restrict recursion
 def _is_list_annotation(annotation: Any) -> bool:
     """Return whether an annotation accepts a list value."""
     if annotation is list or get_origin(annotation) is list:
         return True
+
+    import typing
+    origin = get_origin(annotation)
+    is_union = origin is typing.Union or getattr(origin, "__name__", "") == "UnionType"
+    is_annotated = origin is getattr(typing, "Annotated", type(None))
+    if not (is_union or is_annotated):
+        return False
+
     return any(
         _is_list_annotation(arg)
         for arg in get_args(annotation)
         if arg is not type(None)
     )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@reflexio/server/llm/_litellm_structured_output.py` around lines 56 - 64,
Update _is_list_annotation so recursive traversal occurs only for Union types,
including Optional and Python 3.10 union syntax, and Annotated types; do not
recurse through arbitrary generic arguments such as dict, tuple, or Callable.
Preserve direct list detection while ensuring nested list types inside unrelated
containers return False.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@reflexio/server/llm/_litellm_structured_output.py`:
- Around line 56-64: Update _is_list_annotation so recursive traversal occurs
only for Union types, including Optional and Python 3.10 union syntax, and
Annotated types; do not recurse through arbitrary generic arguments such as
dict, tuple, or Callable. Preserve direct list detection while ensuring nested
list types inside unrelated containers return False.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c2b87068-a678-4028-83f1-675e3ce48afb

📥 Commits

Reviewing files that changed from the base of the PR and between 4519287 and 8045f32.

📒 Files selected for processing (3)
  • reflexio/server/llm/_litellm_structured_output.py
  • tests/e2e_tests/conftest.py
  • tests/server/llm/test_litellm_client_unit.py

@guangyu-reflexio
guangyu-reflexio merged commit eb18b48 into main Jul 18, 2026
1 check passed
@guangyu-reflexio
guangyu-reflexio deleted the fix/e2e-profile-workflow-fixtures branch July 18, 2026 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant