Skip to content

fix(langchain): skip non-dict rows in format_recall_content#2563

Closed
Bartok9 wants to merge 2 commits into
Canner:mainfrom
Bartok9:fix/langchain-format-recall-non-dict-20260722
Closed

fix(langchain): skip non-dict rows in format_recall_content#2563
Bartok9 wants to merge 2 commits into
Canner:mainfrom
Bartok9:fix/langchain-format-recall-non-dict-20260722

Conversation

@Bartok9

@Bartok9 Bartok9 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Skip non-dict recall hits in format_recall_content so mixed memory payloads still render.

Real behavior proof

$ python3 -m pytest sdk/wren-langchain/tests/unit/test_format_recall_content_guard.py -v
2 passed

License

sdk/wren-langchain/** Apache-2.0.

Summary by CodeRabbit

  • Bug Fixes

    • Improved recall result formatting to safely ignore malformed or non-dictionary entries.
    • Kept valid results correctly counted and rendered.
    • Now reliably shows “No similar past queries found.” when no valid results remain after filtering.
  • Tests

    • Added unit tests covering mixed valid/invalid recall rows and the all-invalid case.

Memory recall results may include None/non-dict entries. Guard the
loop so one bad hit does not abort the content envelope.
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 72bf44f5-23da-4896-8404-b90732545f1f

📥 Commits

Reviewing files that changed from the base of the PR and between d6cd764 and 2b64d55.

📒 Files selected for processing (1)
  • sdk/wren-langchain/tests/unit/test_format_recall_content_guard.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • sdk/wren-langchain/tests/unit/test_format_recall_content_guard.py

Walkthrough

format_recall_content skips malformed recall rows, numbers valid entries sequentially, and returns the empty-result message when no valid rows remain. Unit tests cover mixed and entirely invalid inputs.

Changes

Recall formatting

Layer / File(s) Summary
Row validation and tests
sdk/wren-langchain/src/wren_langchain/_format.py, sdk/wren-langchain/tests/unit/test_format_recall_content_guard.py
Non-dictionary rows are skipped, valid rows retain sequential numbering, and mixed or entirely invalid inputs are tested.

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

Possibly related PRs

  • Canner/WrenAI#2521: Updates the same recall formatter with matching invalid-row handling and tests.

Poem

A bunny found bad rows in the hay,
And gently skipped them on the way.
Good queries lined up, one, two, three,
While empty fields said, “None for me!”
Safe recall now hops happily.

🚥 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 matches the main change: guarding format_recall_content against non-dictionary rows.
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 unit tests (beta)
  • Create PR with unit tests

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
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
sdk/wren-langchain/src/wren_langchain/_format.py (1)

114-119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Align the input annotation with the new behavior.

The guard now intentionally accepts mixed inputs, but format_recall_content is still annotated as accepting only list[dict[str, Any]] on Line 107. Update the annotation to reflect non-dictionary rows, such as Sequence[object], so callers and type checkers see the actual contract.

Suggested adjustment
-def format_recall_content(rows: list[dict[str, Any]]) -> str:
+def format_recall_content(rows: Sequence[object]) -> str:
🤖 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 `@sdk/wren-langchain/src/wren_langchain/_format.py` around lines 114 - 119,
Update the input annotation of format_recall_content to represent mixed row
types, such as Sequence[object], matching the existing non-dictionary row guard
while preserving the current rendering behavior.
sdk/wren-langchain/tests/unit/test_format_recall_content_guard.py (1)

11-17: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Assert numbering for every valid row.

This test verifies that the first valid row starts at 1, but it would still pass if subsequent valid rows were also numbered 1. Assert that the second rendered row is numbered 2 to cover the new counter behavior.

Suggested test assertion
-    assert out.startswith('1. "')
+    assert out.startswith('1. "q1"')
+    assert '\n2. "q2"' in out
🤖 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 `@sdk/wren-langchain/tests/unit/test_format_recall_content_guard.py` around
lines 11 - 17, Update test_skips_non_dict_rows to assert that the second valid
rendered row begins with numbering 2, while preserving the existing checks for
skipped non-dictionary rows and rendered query content.
🤖 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 `@sdk/wren-langchain/src/wren_langchain/_format.py`:
- Around line 114-119: Update the input annotation of format_recall_content to
represent mixed row types, such as Sequence[object], matching the existing
non-dictionary row guard while preserving the current rendering behavior.

In `@sdk/wren-langchain/tests/unit/test_format_recall_content_guard.py`:
- Around line 11-17: Update test_skips_non_dict_rows to assert that the second
valid rendered row begins with numbering 2, while preserving the existing checks
for skipped non-dictionary rows and rendered query content.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6164ac76-d4df-4649-befb-2561bc0d4757

📥 Commits

Reviewing files that changed from the base of the PR and between f4b45ed and d6cd764.

📒 Files selected for processing (2)
  • sdk/wren-langchain/src/wren_langchain/_format.py
  • sdk/wren-langchain/tests/unit/test_format_recall_content_guard.py

CI lint fails on ruff format --check for the new unit test.
@Bartok9

Bartok9 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded — non-dict row skipping in format_recall_content already landed on main via #2521 (fix(langchain): skip non-dict rows in format helpers). Branch is DIRTY against current main with no remaining unique change.

@Bartok9 Bartok9 closed this Jul 24, 2026
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