Improve observed SQL matching and docs - #698
Conversation
📝 WalkthroughWalkthroughEnhanced the observed SQL matcher with improved ranking logic (boolean operation normalization, function argument shape comparison), partial failure tolerance for file read/scan failures, and file processing metrics tracking. Updated documentation and tests to reflect new best-effort behavior and skip/warning reporting. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/sql-grep-core/src/observed/match.ts (1)
136-162: Fault-tolerant scanning with appropriate warnings.The try-catch blocks around file read and
splitQueriesensure scanning continues despite individual file failures. Warning codes (file-read-failed,file-scan-failed) distinguish failure modes clearly.One observation:
filesSkippedincrements for both read failures (line 142) and scan failures (line 155). A file that reads successfully but fails to parse will have bothfilesReadincremented (line 140) andfilesSkippedincremented (line 155). This is semantically reasonable since "skipped" means "excluded from candidate pool," but users might initially expectfilesRead + filesSkipped == filesScanned.Consider adding a brief clarification in the docs or a code comment if confusion arises in practice.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sql-grep-core/src/observed/match.ts` around lines 136 - 162, The current loop increments filesRead when readFile succeeds and filesSkipped when splitQueries fails, meaning a single file can be counted in both filesRead and filesSkipped; add a brief inline comment near the loop or the counter declarations (referencing candidateFiles, filesRead, filesSkipped, splitQueries, and warnings) clarifying that filesRead counts files successfully read while filesSkipped counts files excluded from the candidate pool and that these sets may overlap (so filesRead + filesSkipped may exceed total scanned files), or alternatively adjust counters if you prefer invariant filesRead + filesSkipped == totalScanned by either decrementing filesRead on parse failure or introducing a separate filesScanned/parsed counter—pick one approach and document it inline near the existing try/catch blocks for readFile and splitQueries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/sql-grep-core/src/observed/match.ts`:
- Around line 136-162: The current loop increments filesRead when readFile
succeeds and filesSkipped when splitQueries fails, meaning a single file can be
counted in both filesRead and filesSkipped; add a brief inline comment near the
loop or the counter declarations (referencing candidateFiles, filesRead,
filesSkipped, splitQueries, and warnings) clarifying that filesRead counts files
successfully read while filesSkipped counts files excluded from the candidate
pool and that these sets may overlap (so filesRead + filesSkipped may exceed
total scanned files), or alternatively adjust counters if you prefer invariant
filesRead + filesSkipped == totalScanned by either decrementing filesRead on
parse failure or introducing a separate filesScanned/parsed counter—pick one
approach and document it inline near the existing try/catch blocks for readFile
and splitQueries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4acbac20-ad4f-4171-96b7-52e572bb5758
📒 Files selected for processing (7)
docs/guide/observed-sql-investigation.mddocs/guide/observed-sql-matching.mdpackages/sql-grep-core/src/observed/match.tspackages/sql-grep-core/src/observed/types.tspackages/sql-grep-core/tests/observedSqlMatch.test.tspackages/ztd-cli/README.mdpackages/ztd-cli/tests/furtherReading.docs.test.ts
Issue
Customer Value
lower(email)andlower(status)are less likely to collide.Outcome
@rawsql-ts/sql-grep-core.ztd-cliREADME to match the current CLI surface.users/persistencepaths.Acceptance Criteria
Observed SQL Matchingdocs reflect the current behavior and CLI paths.Verification
pnpm --filter @rawsql-ts/sql-grep-core test -- tests/observedSqlMatch.test.tspnpm --filter @rawsql-ts/sql-grep-core buildpnpm --filter @rawsql-ts/ztd-cli test -- tests/furtherReading.docs.test.tsgit diff --checkRepository Evidence
packages/sql-grep-core/src/observed/match.tspackages/sql-grep-core/src/observed/types.tspackages/sql-grep-core/tests/observedSqlMatch.test.tsdocs/guide/observed-sql-matching.mddocs/guide/observed-sql-investigation.mdpackages/ztd-cli/README.mdpackages/ztd-cli/tests/furtherReading.docs.test.tsSupplementary Evidence
sql-grep-coretest suite and theztd-clidocs consistency test passing after the fixes.Open Questions
Merge Blockers
Summary by CodeRabbit
Documentation
ztd query match-observedbehavior, including best-effort matching and file read/skip count reporting with warningsNew Features
Bug Fixes