fix(database): remove dead case_sensitive_like pragma (#1346)#1353
fix(database): remove dead case_sensitive_like pragma (#1346)#1353VanshajPoonia wants to merge 3 commits into
Conversation
PRAGMA case_sensitive_like = ON in get_db_connection() affected zero queries: the only LIKE query (db_get_folder_ids_by_path_prefix) uses a raw sqlite3.connect() that never ran the pragma. Removing it keeps LIKE case-insensitive (SQLite default) and avoids a latent footgun where migrating a LIKE query onto the shared helper would silently turn subfolder prefix matching case-sensitive on macOS/Windows. Add regression tests asserting case-insensitive folder prefix matching.
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughRemoves Fix for latent case_sensitive_like footgun
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/tests/test_folders.py (1)
853-858: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueFixture teardown can leak the fd/temp file if setup fails.
os.close(db_fd)/os.unlink(db_path)run only ifdb_create_folders_table()succeeds and the test reaches teardown. Wrapping cleanup intry/finallymakes the fixture leak-safe.♻️ Leak-safe teardown
db_fd, db_path = tempfile.mkstemp() - with patch.object(folders_module, "DATABASE_PATH", db_path): - folders_module.db_create_folders_table() - yield folders_module - os.close(db_fd) - os.unlink(db_path) + try: + with patch.object(folders_module, "DATABASE_PATH", db_path): + folders_module.db_create_folders_table() + yield folders_module + finally: + os.close(db_fd) + os.unlink(db_path)🤖 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 `@backend/tests/test_folders.py` around lines 853 - 858, The fixture teardown around the temp database in the test setup is not leak-safe because cleanup only runs after db_create_folders_table() succeeds. Update the fixture in test_folders.py to keep the tempfile creation and DATABASE_PATH patching, but wrap the yield and cleanup of db_fd and db_path in a try/finally so os.close and os.unlink always execute even if setup fails.
🤖 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.
Inline comments:
In `@backend/tests/test_folders.py`:
- Around line 839-875: The regression test currently exercises
db_get_folder_ids_by_path_prefix() through a direct SQLite path, so it won’t
catch a case-sensitivity regression in the shared-connection setup. Update
TestFolderPathPrefixMatching to route the prefix lookup through the
shared-connection helper used by app.database.folders (or add a dedicated test
that calls that helper directly) so the query actually covers the connection
path where PRAGMA case_sensitive_like would be applied.
---
Nitpick comments:
In `@backend/tests/test_folders.py`:
- Around line 853-858: The fixture teardown around the temp database in the test
setup is not leak-safe because cleanup only runs after db_create_folders_table()
succeeds. Update the fixture in test_folders.py to keep the tempfile creation
and DATABASE_PATH patching, but wrap the yield and cleanup of db_fd and db_path
in a try/finally so os.close and os.unlink always execute even if setup fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c861a4ae-10d0-43b7-95cf-cbcfb16f4cd5
📒 Files selected for processing (2)
backend/app/database/connection.pybackend/tests/test_folders.py
💤 Files with no reviewable changes (1)
- backend/app/database/connection.py
CodeRabbit flagged that the existing regression test for AOSSIE-Org#1346 only exercises db_get_folder_ids_by_path_prefix(), which opens SQLite directly and never goes through get_db_connection(). Add a test that calls the shared helper directly so a future PRAGMA case_sensitive_like = ON reintroduced there is caught immediately, even before any production query relies on it.
os.close(db_fd) and os.unlink(db_path) only ran after db_create_folders_table() succeeded, so a setup failure would leak the fd and temp file. Wrap teardown in try/finally so cleanup always runs.
1 participant
PRAGMA case_sensitive_like = ON in get_db_connection() affected zero queries: the only LIKE query (db_get_folder_ids_by_path_prefix) uses a raw sqlite3.connect() that never ran the pragma. Removing it keeps LIKE case-insensitive (SQLite default) and avoids a latent footgun where migrating a LIKE query onto the shared helper would silently turn subfolder prefix matching case-sensitive on macOS/Windows.
Add regression tests asserting case-insensitive folder prefix matching.
Addressed Issues:
Fixes #1346
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: Codex
Checklist
Summary by CodeRabbit
Bug Fixes
LIKEbehavior from connection setup, which may affect path and text matching consistency.Tests