Skip to content

chore: drop dead type-stubs + fix citation test transaction nesting#263

Merged
guangyu-reflexio merged 1 commit into
mainfrom
cleanup/post-decomposition
Jul 1, 2026
Merged

chore: drop dead type-stubs + fix citation test transaction nesting#263
guangyu-reflexio merged 1 commit into
mainfrom
cleanup/post-decomposition

Conversation

@guangyu-reflexio

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

Copy link
Copy Markdown
Contributor

Post-decomposition cleanup: dead type-stubs + a brittle-test fix

Small follow-up to the playbook decomposition (#262).

  • Remove orphaned MRO type-stub attributes that the mixin split left unused in their sub-mixin (sqlite_storage/playbook/_agent.py, _user.py). Type-only annotations with zero runtime effect; each removed stub was verified unused in its file and pyright stays clean. (Surfaced by the whole-branch review.)
  • Fix test_get_citations_by_session_ids_extracts_cited_rows (pre-existing brittle test): it ran a raw storage.conn.execute("UPDATE ...") on the shared connection without committing, leaving an implicit transaction open, so the next add_request()'s BEGIN IMMEDIATE raised "cannot start a transaction within a transaction." Commit the raw UPDATEs.

Full OSS storage suite: 844 passed, 0 failures (this was the last remaining red). pyright + ruff clean.

Summary by CodeRabbit

  • Bug Fixes
    • Improved SQLite data handling so updates are reliably saved before citation lookups run, reducing inconsistent results in edge cases.
    • Kept playbook storage behavior aligned with current SQLite capabilities, helping avoid unexpected issues during search and delete operations.
  • Tests
    • Updated test coverage to reflect committed database changes before citation retrieval checks.

- Remove orphaned MRO type-stub attributes left by the playbook decomposition
  that are now unused in their sub-mixin (sqlite _agent.py: _fts_delete/_vec_delete/
  _has_sqlite_vec/_subject_ref_for_user_id/_assert_subject_writable_locked;
  _user.py: _execute/_fts_delete/_vec_delete/_has_sqlite_vec). Type-only, pyright clean.
- test_get_citations_by_session_ids_extracts_cited_rows: commit the raw UPDATE on
  the shared connection so the following add_request()'s BEGIN IMMEDIATE no longer
  raises 'cannot start a transaction within a transaction'.
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: dbcfd8f0-7e5d-46f2-a65b-192af11d0a7d

📥 Commits

Reviewing files that changed from the base of the PR and between b33330f and e0a3110.

📒 Files selected for processing (3)
  • reflexio/server/services/storage/sqlite_storage/playbook/_agent.py
  • reflexio/server/services/storage/sqlite_storage/playbook/_user.py
  • tests/server/services/storage/test_sqlite_storage_bc_extras.py
💤 Files with no reviewable changes (2)
  • reflexio/server/services/storage/sqlite_storage/playbook/_agent.py
  • reflexio/server/services/storage/sqlite_storage/playbook/_user.py

📝 Walkthrough

Walkthrough

Removes now-unused class-level type-hint attributes (_fts_delete, _vec_delete, _execute, _has_sqlite_vec) from AgentPlaybookStoreMixin and UserPlaybookStoreMixin, replacing them with other helper hints where applicable. A test is updated to explicitly commit the SQLite connection after direct citation UPDATE statements.

Changes

Playbook Mixin and Test Fix

Layer / File(s) Summary
Type-hint cleanup in playbook mixins
reflexio/server/services/storage/sqlite_storage/playbook/_agent.py, reflexio/server/services/storage/sqlite_storage/playbook/_user.py
Removes unused type-hint attribute declarations (_fts_delete, _vec_delete, _execute, _has_sqlite_vec) from both mixins; _user.py also adds explicit hints for _fetchone, _fetchall, _get_embedding, _should_expand_documents, _expand_document, _fts_upsert, _vec_upsert, _delete_playbook_search_rows.
Citations test commit fix
tests/server/services/storage/test_sqlite_storage_bc_extras.py
Adds explicit storage.conn.commit() calls after two direct UPDATE interactions SET citations = ... statements before subsequent inserts and the final assertion call.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

🚥 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 two main changes: removing dead type stubs and fixing the citation test transaction issue.
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 cleanup/post-decomposition

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.

@guangyu-reflexio guangyu-reflexio merged commit 21d1167 into main Jul 1, 2026
1 check passed
@guangyu-reflexio guangyu-reflexio deleted the cleanup/post-decomposition branch July 1, 2026 04:01
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