feat(describegpt): --two-pass cross-field Data Dictionary refinement#3863
Merged
Conversation
Adds an opt-in `--two-pass` flag that runs a second LLM call after the
first dictionary pass, feeding the entire first-pass dictionary back to
the LLM via a new `{{ first_pass_dictionary }}` template variable.
Cross-field context lets the LLM recognize groups of related fields
(street_no + street_name + city + state + zip describing one mailing
address; first_name + last_name naming one person; lat + lng forming a
coordinate pair) and refine Label, Description and Content Type
accordingly. The refined dictionary becomes the emitted output and is
what downstream description / tags / prompt phases see as dictionary
context.
Key invariants:
- baseline-preserving merge: if the refine LLM omits a field, the
first-pass Label/Description survive (the alternative would silently
wipe them back to code-derived defaults)
- deterministic `unique_id` stamp survives the refine pass
- LLM-supplied `unique_id` rejected in both baseline and refine passes
- distinct cache entry under `PromptType::DictionaryRefine`, keyed off
the first-pass dictionary fingerprint; cascade invalidation from
`Dictionary` -> `DictionaryRefine`
- mutually exclusive with `--prepare-context` / `--process-response`
(MCP sampling is single-turn per phase)
- backward compat: `dictionary_refine_prompt` is `#[serde(default)]` so
existing user `--prompt-file` TOMLs keep parsing
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 17 |
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.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in --two-pass mode to qsv describegpt that runs a second “refine” LLM call using the full first-pass Data Dictionary as context, then merges baseline + refine results so omitted fields don’t regress to code-derived defaults.
Changes:
- Introduces
PromptType::DictionaryRefineplus a newdictionary_refine_prompttemplate (with#[serde(default)]fallback for older user prompt TOMLs). - Implements a two-pass dictionary merge (
combine_dictionary_entries_with_baseline) and wires a two-pass flow inrun_dictionary_phase(including separate caching for the refine pass). - Updates defaults/docs to document
--two-passand ship the refine prompt template.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/cmd/describegpt/dictionary.rs | Adds baseline-preserving merge function for two-pass output + unit tests for invariants. |
| src/cmd/describegpt.rs | Adds --two-pass, DictionaryRefine prompt type, first-pass dictionary context plumbing, refine caching logic, and two-pass execution path. |
| resources/describegpt_defaults.toml | Adds dictionary_refine_prompt default template and documents {{ first_pass_dictionary }}. |
| docs/help/describegpt.md | Regenerates help to include the new --two-pass flag. |
Addresses two roborev review findings on the --two-pass change: - Wrap FIRST_PASS_DICT_JSON seeding in a `FirstPassDictGuard` RAII type so the slot is cleared on all exit paths from `run_dictionary_phase`, including `?`-propagated errors from the refine LLM call. Replaces the manual seed/clear pair which would leave stale state on the error path (harmless in CLI one-shot, but a leak in the test-harness / repeated-invocation case the static's doc comment calls out). - Make the `get_prompt` and `get_cache_key_with_flag` reads `.expect()` on poison to match the write side's behavior. Previously the reads silently coerced a poisoned RwLock to an empty `first_pass_dictionary` / empty fingerprint, which would waste the refine LLM call (no cross-field context) with no signal to the user. Now a poisoned lock fails loud, matching `FirstPassDictGuard::seed()`'s `.expect()`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… refine cache Copilot review on PR #3863 caught that `build_first_pass_dictionary_json_string` was expanding `{GENERATED_BY_SIGNATURE}` via `replace_attribution_placeholder`, which injects a live `chrono::Utc::now()` timestamp and the full process `command_line`. That made `FIRST_PASS_DICT_JSON` change on every invocation even for byte-identical first-pass dictionary content, busting the `DictionaryRefine` cache key (which is BLAKE3-fingerprinted from that string) and forcing a fresh second LLM call on every run. Fix: drop the attribution field outright when building the first-pass JSON. The attribution is for the user-facing emit path; the LLM doesn't need it and keeping it in the cache-fingerprint input only adds noise. The unused `model`/`base_url` parameters are removed from the function signature as a result. Adds `first_pass_dictionary_json_is_stable_across_invocations` as a regression guard — calls the builder twice with a 10ms sleep between calls and asserts byte-identical output plus absence of both `attribution` and the literal `{GENERATED_BY_SIGNATURE}` placeholder. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
--two-passflag toqsv describegptthat runs a second LLM call after the first dictionary pass, feeding the entire first-pass Data Dictionary back to the LLM as{{ first_pass_dictionary }}context so it can refine Label, Description and (when--infer-content-typeis set) Content Type using cross-field awareness.street_no+street_name+city+state+zip→ mailing address;first_name+last_name→ person name;lat+lng→ coordinate pair; date + time → split timestamp) and label/describe them with that context, rather than field-by-field in isolation.How it works
PromptType::DictionaryRefinevariant gives the refine pass its own prompt template (dictionary_refine_promptinresources/describegpt_defaults.toml) and its own cache entry. Cache key for the refine entry is derived from a BLAKE3 fingerprint of the first-pass dictionary, so a different first-pass output misses the cache.FIRST_PASS_DICT_JSON: RwLock<Option<String>>global surfaces the first-pass dictionary to the refine prompt's{{ first_pass_dictionary }}template variable.combine_dictionary_entries_with_baseline()merges baseline + refine LLM responses so that any field the refine pass omits inherits the first-pass Label/Description rather than reverting to code-derived defaults. The deterministicunique_idstamp and the LLM-unique_id-rejection defenses are preserved across both passes.{{ dictionary }}context for downstream--description,--tagsand--promptphases.Compatibility and validation
--prepare-contextand--process-response: MCP sampling is single-turn per phase and the refine prompt depends on the first-pass response, so combining them errors out. Documented in USAGE.--no-cache— every invocation pays both LLM calls in that mode.dictionary_refine_promptis#[serde(default = "default_dictionary_refine_prompt")]onPromptFile, so existing user--prompt-fileTOMLs continue to parse and get the built-in template as a fallback. A test enforces byte-identity between the TOML block and the const fallback.--forget --dictionary(and--forgetwith no inference flag) now also purges theDictionaryRefinecache entry.Tests added
combine_with_baseline_preserves_baseline_when_refine_omits_field— the core correctness invariant.combine_with_baseline_rejects_refine_supplied_unique_id— deterministic stamp survives refine; LLMunique_idcan't be smuggled in for non-ALL_UNIQUE fields.combine_with_baseline_refine_overrides_valid_baseline_content_type— the point of the feature: refine can upgradefree_text→address_streetonce cross-field context is visible.combine_with_baseline_matches_single_pass_when_refine_empty— sanity check that the new merge function agrees with the single-pass combine when refine is empty.dictionary_refine_cache_key_differs_from_dictionary_cache_key,two_pass_flag_does_not_change_first_pass_cache_key,dictionary_refine_cache_key_reflects_first_pass_dict_json— cache isolation between passes and dependency on first-pass content.dictionary_refine_prompt_renders_first_pass_dictionary_var— end-to-end prompt rendering of{{ first_pass_dictionary }}.prompt_file_without_dictionary_refine_prompt_field_still_parses— backward compat for pre-existing user prompt files.default_dictionary_refine_prompt_matches_resource— keeps the#[serde(default)]const in sync with the TOML default.Test plan
cargo build --locked --bin qsv -F all_featurescargo t describegpt -F all_features— 64 passed, 0 failedcargo clippy -F all_features— cleancargo +nightly fmt./target/debug/qsv --generate-help-md—docs/help/describegpt.mdregenerated with the new--two-passrowqsv describegpt sample.csv --dictionary --two-pass --prepare-context— confirm clear errorqsv describegpt sample.csv --dictionary --two-pass --no-cache— confirm warning prints--freshand confirm both pass-1 and pass-2 cache entries refresh in~/.qsv-cache/describegpt--prompt-file custom.tomlwherecustom.tomllacksdictionary_refine_promptand confirm theserde(default)fallback works🤖 Generated with Claude Code