fix(describegpt): honor QSV_LLM_BASE_URL env var; repair stale describegpt tests#3889
Merged
Conversation
Fixes 5 describegpt integration tests that fail under QSV_TEST_DESCRIBEGPT=1 against a live local LLM. Real bug - QSV_LLM_BASE_URL was silently ignored: The `DEFAULT_BASE_URL` sentinel (`https://api.openai.com/v1`) had drifted from the actual `--base-url` docopt default (`http://localhost:1234/v1`). The base-URL precedence checks compare `flag_base_url` against that sentinel to distinguish "user passed --base-url" from "docopt default in effect", so the comparison was always true: describegpt always used the docopt default and the QSV_LLM_BASE_URL env-var branch was dead code. Realign the constant (and the default prompt file's `base_url`) with the docopt default so the intended explicit-flag > env-var > default precedence actually works. (fixes describegpt_baseurl_precedence_env_over_default) Test repairs: - The hand-written stats.csv fixture shared by describegpt_stats_options_file_prefix and describegpt_both_file_prefixes had an empty `nullcount` cell in both rows; `parse_stats_csv` parses nullcount strictly. Filled in the missing 0s. - describegpt_empty_dataset expected success on a header-only CSV, but describegpt's first step (`stats`) cannot analyze a zero-row file. Changed it to assert the (correct) error. - describegpt_score_high_threshold_triggers_retries accepted only two of the retry-path messages; broadened it to all of them, since which one appears depends on the non-deterministic refined query. Verified against a live local LLM: the 5 tests now pass and the full describegpt suite is regression-free (the one intermittent failure, describegpt_jsonschema_dictionary, is pre-existing LLM-output flakiness and passes on re-run). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 7 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
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
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.
Fixes 5
describegptintegration tests that fail underQSV_TEST_DESCRIBEGPT=1against a live local LLM. Surfaced while running the test suite for #3888; unrelated to that PR, so split out here.Real bug —
QSV_LLM_BASE_URLwas silently ignoredThe
DEFAULT_BASE_URLconstant (https://api.openai.com/v1) had drifted from the actual--base-urldocopt default (http://localhost:1234/v1). Two base-URL precedence checks compareflag_base_urlagainst that sentinel to tell "user explicitly passed--base-url" from "docopt default in effect". Because the sentinel never matched the real default, the comparison was always true — describegpt always took the "explicit flag" branch, used the docopt default, and never consultedQSV_LLM_BASE_URL. The env-var precedence branch was dead code.Fix: realign
DEFAULT_BASE_URL(and the default prompt file'sbase_url) with the docopt default, so the intended explicit--base-url>QSV_LLM_BASE_URL> default precedence works. Added a comment tying the constant to the docopt default so it can't drift again.Fixes
describegpt_baseurl_precedence_env_over_default.Test repairs
describegpt_stats_options_file_prefix/describegpt_both_file_prefixes— the hand-writtenstats.csvfixture had an emptynullcountcell in both data rows;parse_stats_csvparsesnullcountstrictly and errored. Filled in the missing0s.describegpt_empty_dataset— expected success on a header-only CSV, but describegpt's first step (stats) cannot compile statistics for a zero-row file. Changed it to assert the (correct) error.describegpt_score_high_threshold_triggers_retries— accepted only 2 of the retry-path messages; broadened to all of them, since which message appears depends on the non-deterministic refined query.Verification
Run against a live local LLM (LM Studio,
QSV_TEST_DESCRIBEGPT=1):describegpt_jsonschema_dictionaryfails intermittently under sustained serial load — pre-existing LLM-output flakiness, passes on re-run.)🤖 Generated with Claude Code