refactor: describegpt enable disk cache by default, etc.
#2951
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- improve usage text with a dedicated Caching section - updated examples - changed default enum-threshold from 50 to 20 - added `-p` shortcut for `--prompt` - added enclosing brackets to INPUT_TABLE_NAME to minimize mismatches - added dedicated envar for describegpt Redis connection string QSV_DG_REDIS_CONNSTR - added delimiter to AnalysisResults struct - changed get_prompt function to use AnalysisResults struct as an Option - fine-tuned DuckDB SQL generation guidelines to minimize invalid SQL - change cache keys, eliminating args.arg_input and making file_hash the first field, this should increase cache hits for the same file, even if they're in different locations - make DuckDB INPUT_TABLE_NAME replacement more robust, by using regex, rather than a simple contains/replace - replaced nested if for cache processing with more maintainable match statement
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the describegpt command to enable disk cache by default, improve the user experience, and enhance SQL generation capabilities. The changes focus on making caching more user-friendly while providing better guidance for SQL query generation.
Key changes include:
- Enable disk cache by default instead of requiring explicit opt-in
- Improved usage documentation with dedicated caching section
- Enhanced SQL generation guidelines for DuckDB with more robust table name replacement
- Streamlined cache key generation for better cache hit rates
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/cmd/describegpt.rs | Main refactoring with default disk cache, improved SQL guidelines, better cache key generation, and enhanced error handling |
| docs/ENVIRONMENT_VARIABLES.md | Added documentation for new QSV_DG_REDIS_CONNSTR environment variable |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
-pshortcut for--prompt