Skip to content

refactor(adapter): remove obsolete Reflexio compatibility shims#137

Merged
yyiilluu merged 1 commit into
ReflexioAI:mainfrom
wenchanghan:codex/adapter-cleanup-safe
Jul 16, 2026
Merged

refactor(adapter): remove obsolete Reflexio compatibility shims#137
yyiilluu merged 1 commit into
ReflexioAI:mainfrom
wenchanghan:codex/adapter-cleanup-safe

Conversation

@wenchanghan

@wenchanghan wenchanghan commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Why

claude-smart releases vendor a current Reflexio source snapshot. The supported vendored client already guarantees the timeout, publish keywords, and list APIs that the adapter was probing for, so those compatibility branches were unreachable in shipped installs and made the wrapper harder to reason about.

What changed

  • construct ReflexioClient with the hook-safe timeout directly
  • always forward source and override_learning_stall
  • use get_user_playbooks and get_agent_playbooks directly for broad reads
  • remove obsolete compatibility tests and update current-contract test doubles

The raw publish path and the 1,000-link truncation guard remain intentionally: the public client still cannot accept a caller request ID, and removing the cap would turn oversized publishes into whole-request validation failures.

Validation

  • focused adapter suite: 47 passed
  • full claude-smart suite: 683 passed, 1 skipped
  • Ruff and scoped Pyright: passed
  • OpenCode TypeScript build and Node syntax checks: passed
  • npm tarball built successfully with Reflexio upstream/main vendored; required vendored files verified

Summary by CodeRabbit

  • Bug Fixes
    • Improved compatibility with the latest Reflexio client API.
    • Playbook retrieval now uses consistent result limits.
    • Publishing requests now reliably include source and learning-stall override settings.
    • Configured request timeouts are applied consistently.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 20b99156-bb7f-4114-970c-a0180775b2ec

📥 Commits

Reviewing files that changed from the base of the PR and between 3a34a1f and 3f8c266.

📒 Files selected for processing (2)
  • plugin/src/claude_smart/reflexio_adapter.py
  • tests/test_adapter.py

📝 Walkthrough

Walkthrough

The adapter now requires timeout-aware ReflexioClient construction, always includes publish compatibility fields, and exclusively uses get_*_playbooks with limit for playbook retrieval. Tests and client stubs are updated accordingly.

Changes

Reflexio client contract alignment

Layer / File(s) Summary
Client constructor contract
plugin/src/claude_smart/reflexio_adapter.py, tests/test_adapter.py
Client construction passes timeout directly; keyword introspection and fallback coverage for older client constructors are removed.
Publish request contract
plugin/src/claude_smart/reflexio_adapter.py, tests/test_adapter.py
Publish calls always include source and override_learning_stall; tests for omitting these fields under older contracts are removed.
Playbook retrieval contract
plugin/src/claude_smart/reflexio_adapter.py, tests/test_adapter.py
User and agent playbook retrieval always uses get_*_playbooks(limit=...), with test doubles and default parameter assertions updated.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: yilu331, yyiilluu

🚥 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: removing obsolete Reflexio compatibility shims in the adapter.
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.

@wenchanghan wenchanghan left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Code Review Summary

I have reviewed the changes in PR #137: "refactor(adapter): remove obsolete Reflexio compatibility shims" and verified them against the test suite.

Findings & Observations:

  1. Compatibility Shim Removal:
    • Constructor Timeout: Removed the try...except TypeError block when initializing ReflexioClient with timeout=_HTTP_TIMEOUT_SECONDS. This is clean and correct because the pinned Reflexio client version (0.2.28 or newer) always supports the timeout parameter.
    • Publish Interaction Parameters: Removed dynamic checking of keyword arguments (supports_source / _supports_keyword). The adapter now directly forwards source and override_learning_stall as they are standard in modern client interactions.
    • Playbook Methods: Replaced fallback logic (search_user_playbooks / search_agent_playbooks) with direct calls to get_user_playbooks and get_agent_playbooks using limit=top_k.
    • Test Cleanups: Deleted obsolete tests for old compatibility fallbacks (test_adapter_falls_back_when_client_rejects_timeout_kwarg, test_publish_omits_override_learning_stall_when_client_lacks_keyword, test_raw_publish_omits_source_for_older_client_contract).
  2. Metadata & Locks:
    • Correctly bumped version from 0.2.49 to 0.2.50 across package.json, README.md, plugin/.claude-plugin/plugin.json, plugin/.codex-plugin/plugin.json, and plugin/pyproject.toml.
    • Updated the locked commit reference in reflexio.lock.json to lock the upgraded Reflexio version (0.2.28 commit 26bed34).
  3. Local Testing Verification:
    • Ran the full test suite (uv run pytest) on the PR branch changes.
    • Result: 684 passed. All tests passed successfully.

Conclusion:

The code cleanup successfully simplifies the adapter layer by removing unreachable compatibility paths for deprecated versions. This reduces code complexity and maintenance overhead. The changes are fully verified and safe to merge.

@wenchanghan

Copy link
Copy Markdown
Contributor Author

Two-axis review — 3a34a1fc...3f8c266

Pinned fixed point: 3a34a1fc5ea591f5236aa7f631cb685901f3a645 (origin/main). HEAD: 3f8c266a9ed01fab10006561340aeeb8a6f044bd. 1 commit, 2 files, +33 / −176. Ran Standards and Spec as parallel sub-agents against the full diff.

Standards

Documented sources consulted: pythonic-code.md, python-docstrings.md, reflexio-patterns.md (scoped to reflexio/** — does not apply to this path), DEVELOPER.md.

(a) Documented-standard violations introduced by the diff: none. The diff is delete-only against the production module; remaining except Exception blocks keep their existing # noqa: BLE001 with justification. No new public functions, no new docstrings, no new comments — pythonic-code.md, python-docstrings.md, and the host AGENTS "no comments unless asked" rule all pass.

(b) Baseline smells: the diff removes smells rather than adding them.

  • Speculative Generality — removed. try/except TypeError around ReflexioClient(timeout=...), the if … elif override_learning_stall fallback, the hasattr(client, "get_*_playbooks") branches, and the _supports_keyword introspection helper are all dead code against plugin/vendor/reflexio/reflexio/client/client.py:161 (timeout: int = 300), :398 (source/override_learning_stall), :1464 (get_user_playbooks(limit=…)), :1756 (get_agent_playbooks(limit=…)). Deletion is correct.
  • Middle Man — removed. _supports_keyword was a thin wrapper around inspect.signature; inlining-by-deletion is the textbook fix.

Judgement call (minor, judgement only — tooling won't flag): the test fakes now require timeout: int with no default (tests/test_adapter.py:71,128,182,222). The vendored ReflexioClient.__init__ defaults timeout=300. Mirroring that default in the fakes (timeout: int = 5) would keep the doubles more honest. Not blocking.

Spec

Spec source: PR body "Why" / "What changed" (no linked issue, no PRD, no separate spec file). Verification cross-checked against the vendored reflexio client.

(a) Missing or partial: none. All four "What changed" bullets are implemented:

  • ReflexioClient(... timeout=_HTTP_TIMEOUT_SECONDS ...) direct, inner TypeError fallback gone — reflexio_adapter.py:94-100.
  • source and override_learning_stall always forwarded — reflexio_adapter.py:139,182-184.
  • get_user_playbooks / get_agent_playbooks called directly with limit=top_kreflexio_adapter.py:336-343,364-371.
  • Obsolete tests removed (test_adapter_falls_back_when_client_rejects_timeout_kwarg, test_publish_omits_override_learning_stall_when_client_lacks_keyword, test_raw_publish_omits_source_for_older_client_contract); test doubles renamed at tests/test_adapter.py:42-45, 542-545, 666-669, 687-690, 797.

(b) Scope creep: none. Beyond the four bullets the diff only drops the now-unused import inspect and the dead _supports_keyword helper — both mechanical consequences of the stated removals.

(c) Wrong-looking implementation: none. Stated invariants that must survive the cleanup are intact:

  • Raw path: getattr(client, "_make_request", None) and the request_id raw POST remain at reflexio_adapter.py:126-164.
  • 1,000-link truncation guard: _needs_raw_retrieved_learning_publish and _without_retrieved_learnings still defined and still called in both the raw and public branches (reflexio_adapter.py:155-166, 170-174, 521-537).
  • Public-client contract: non-raw branch calls client.publish_interaction(...) without request_id (reflexio_adapter.py:184-185), so "the public client still cannot accept a caller request ID" still holds.

Summary

  • Standards: 0 hard findings, 1 minor judgement call (test-fake timeout default). Worst issue within axis: the test-fake signature noise.
  • Spec: 0 findings across missing / scope creep / wrong-implementation. Worst issue within axis: none.

@wenchanghan

Copy link
Copy Markdown
Contributor Author

Review feedback reconciliation

Reviewed the posted MiniMax/Gemini feedback and completed an independent clean-context Terra High pass.

Feedback Decision Rationale
Give the test-double timeout parameter a default Not applied Requiring the keyword is intentional regression coverage: these doubles should fail if Adapter stops forwarding the hook-safe timeout. The focused timeout test also asserts the exact value. A default would make the double less sensitive to that contract.
Version 0.2.50 and reflexio.lock.json observations No change Those files are the origin/main baseline, not part of this PR diff. The PR remains limited to the adapter and its tests.
Compatibility removals are valid against the vendored Reflexio contract Accepted No additional change needed; this is the behavior implemented and covered by the focused and full suites.
Terra High clean-context review No findings Independent review inspected the diff and surrounding vendoring, correctness, regression, maintainability, and test contracts without reading prior review comments.
CodeRabbit No actionable findings No change needed.

Net: all feedback is incorporated; no source amendment is justified. The existing commit and green CI remain the review candidate.

@yyiilluu yyiilluu merged commit 60a2bd8 into ReflexioAI:main Jul 16, 2026
9 checks passed
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.

2 participants