feat(fuzzy-finder): statement name + argument completion framework#507
feat(fuzzy-finder): statement name + argument completion framework#507
Conversation
…VARIABLE Add fuzzy finder support for SET and SHOW VARIABLE statements. Typing `SET CLI_<Ctrl+T>` or `SHOW VARIABLE <Ctrl+T>` opens the fuzzy finder with all registered system variable names. Zero API calls needed as variable names come from the in-memory registry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello @apstndb, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the command-line interface by introducing intelligent fuzzy completion for system variable names. Users can now leverage fuzzy search when typing Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request successfully adds fuzzy finder support for system variable names in SET and SHOW VARIABLE contexts. The implementation is well-integrated with the existing fuzzy finder infrastructure and includes comprehensive tests. I identified one high-severity issue where the completion feature would fail on its first use if the system variable registry hadn't been lazily initialized yet.
ListVariables() internally calls ensureRegistry(), making the explicit Registry nil check redundant. The check also prevented the fuzzy finder from working on first use when the registry hadn't been initialized yet. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code Metrics Report📊 View detailed coverage report (available for 7 days)
Details | | main (1292e55) | #507 (1f7e7bc) | +/- |
|---------------------|----------------|----------------|-------|
- | Coverage | 69.3% | 68.8% | -0.5% |
| Files | 55 | 55 | 0 |
| Lines | 5762 | 5843 | +81 |
+ | Covered | 3997 | 4024 | +27 |
+ | Code to Test Ratio | 1:1.2 | 1:1.2 | +0.0 |
| Code | 13244 | 13432 | +188 |
+ | Test | 16622 | 16859 | +237 |
+ | Test Execution Time | 2m6s | 1m18s | -48s |Code coverage of files in pull request scope (68.0% → 59.4%)
Reported by octocov |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable enhancement by adding fuzzy finder support for system variable completion in SET and SHOW VARIABLE contexts. The implementation is straightforward, and the added tests provide good coverage for the new functionality and its edge cases. However, I've identified a critical concurrency issue. The new code makes a call to a function that is not thread-safe, which introduces a data race risk. This violates the repository's concurrency guidelines and should be addressed to ensure application stability.
…ompletion framework Unify fuzzy completion with clientSideStatementDef so both statement names and arguments are completable from a single source of truth. Two completion modes in one Ctrl+T press: - Argument completion (priority): if input matches a statement with a Completion entry (e.g. "USE ", "SET ", "SHOW COLUMNS FROM "), complete the argument - Statement name completion (fallback): show all client-side statement syntaxes as fuzzy candidates (e.g. typing "SHO" shows all SHOW-prefixed statements) Key changes: - Add fuzzyArgCompletion type and Completion field to clientSideStatementDef - Add Completion entries for USE, SET, SHOW VARIABLE, SHOW COLUMNS FROM, SHOW INDEX FROM, TRUNCATE TABLE, DROP DATABASE - Add fetchTableCandidates querying INFORMATION_SCHEMA.TABLES - Rewrite detectFuzzyContext to iterate defs instead of hardcoded regexes - Add extractFixedPrefix to derive insert text from syntax strings - Use cursor position (not full buffer) for context detection - Show loading indicator with timeout for network-dependent completions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request introduces a comprehensive fuzzy completion framework, unifying statement name and argument completion. It integrates completion logic directly into clientSideStatementDef and adds support for database, variable, and table name completion. The changes also include a loading indicator for network-dependent fetches and cursor-position-aware context detection. The test suite has been expanded to cover the new completion logic. The changes are well-structured and align with the project's goals of enhancing user experience through improved CLI interactivity. The code adheres to the Go best practices outlined in the repository style guide, particularly regarding clarity and maintainability in test code.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request introduces a robust fuzzy completion framework for client-side statements, unifying statement name and argument completion. It correctly leverages clientSideStatementDef for completion logic and adds support for database, variable, and table name completion. The implementation includes a loading indicator for network-dependent fetches and uses cursor position for context detection, which are good usability improvements. The test coverage for detectFuzzyContext and extractFixedPrefix is comprehensive. Overall, the changes significantly enhance the interactive experience of the CLI.
- Use FindStringSubmatchIndex for argStart calculation in detectFuzzyContext, replacing the implicit suffix convention with direct byte position from the regex engine. - Improve fuzzyCompletionType.String() default case to include the numeric value for easier debugging of unhandled types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request introduces a comprehensive fuzzy completion framework for client-side statements, unifying statement name and argument completion. It correctly leverages the clientSideStatementDef structure to define completion types and patterns, eliminating redundant regexes. The implementation includes a loading indicator for network-dependent completions and correctly uses the cursor position for context detection. The test coverage for detectFuzzyContext has been significantly expanded to cover various argument completion contexts and statement name fallbacks, which is a great improvement. Additionally, new tests for extractFixedPrefix, buildStatementNameCandidates, and statementNameDisplayTexts ensure the robustness of the statement name completion logic. Overall, the changes are well-structured and enhance the user experience by providing intelligent and context-aware completions. One comment's severity was adjusted to reflect that the identified issue is for future robustness rather than an immediate problem, aligning with existing guidelines.
|
/gemini summary |
Summary of ChangesThis pull request significantly enhances the interactive command-line experience by overhauling the fuzzy completion system. It unifies the logic for suggesting both statement names and their arguments, making the completion process more intelligent and extensible. Users will now benefit from context-aware suggestions for various SQL commands, including databases, variables, and table names, along with a robust fallback to general statement name completion. The changes also introduce visual feedback for network-intensive operations and improve the accuracy of completion based on the cursor's position. Highlights
Changelog
Activity
|
Improve review workflow skills and Gemini style guide based on learnings from PR #507 review cycles. Key changes: - review-cycle.md: Expand from simple delegation to fully autonomous workflow with thread-type detection, fix/explain/acknowledge strategies, and explicit reply-then-re-review loop (fixes logical gap in step 6) - review-respond.md: Add reply content guidelines (no bare commit hashes), improve examples with explanation-first format, fix arguments field typo - .gemini/styleguide.md: Add "Avoid Praise-Only Comments" section to reduce noise (~30% of #507 threads were praise-only)
Summary
clientSideStatementDef— both statement names and arguments completable from a single source of truthCompletionfield toclientSideStatementDef, eliminating parallel maintenance of separate regexes infuzzy_finder.goINFORMATION_SCHEMA.TABLESTwo completion modes in one Ctrl+T press
Completionentry, complete the argumentBehavior examples
Completable argument contexts
USE <db>ListDatabasesRPCDROP DATABASE <db>ListDatabasesRPCSET <var>SHOW VARIABLE <var>SHOW COLUMNS FROM <table>INFORMATION_SCHEMA.TABLESSHOW INDEX FROM <table>INFORMATION_SCHEMA.TABLESTRUNCATE TABLE <table>INFORMATION_SCHEMA.TABLESTest plan
make checkpassesTestDetectFuzzyContextcovers: all arg completion contexts, statement name fallback, edge cases (SET with =, USE without space)TestExtractFixedPrefixcovers: no-arg, single-arg, multi-keyword, optional, curly brace, ellipsisTestBuildStatementNameCandidatesandTestStatementNameDisplayTexts🤖 Generated with Claude Code