Repository navigation
Conversation
…, conflicts from data dicts
…mns from schema card
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
R omits value-label annotations for numeric-coded columns, losing their meanings in the LLM schema.
1 open finding
What changed in this PR
Extends the data-dictionary alignment from #330, surfacing newer metadata in Python and R prompts and schema views.
Changes:
- Adds descriptive annotations, value labels, examples, and table constraints.
- Hides restricted columns from schema cards while retaining annotated LLM context.
- Passes relationship metadata into prompts and adds tests and release notes.
| File | Description |
|---|---|
| pkg-r/tests/testthat/test-querychat_tools.R | Tests annotations and restricted-column hiding. |
| pkg-r/tests/testthat/test-DataDict.R | Tests prompt metadata pass-through. |
| pkg-r/R/TblSqlSource.R | Passes table specifications to formatting. |
| pkg-r/R/QueryExecutor.R | Passes table specifications to formatting. |
| pkg-r/R/querychat_tools.R | Filters restricted schema-card columns. |
| pkg-r/R/DBISource.R | Extracts and formats new metadata. |
| pkg-r/R/DataDict.R | Preserves table labels and relationship metadata. |
| pkg-r/NEWS.md | Documents new metadata support. |
| pkg-py/tests/test_tools.py | Tests constraints and restricted-column hiding. |
| pkg-py/tests/test_datasource.py | Tests schema annotations. |
| pkg-py/tests/test_data_dict.py | Tests parsing and metadata propagation. |
| pkg-py/src/querychat/tools.py | Adds constraints and filters schema cards. |
| pkg-py/src/querychat/_datasource.py | Stores and formats new column metadata. |
| pkg-py/src/querychat/_data_dict.py | Parses and propagates new dictionary fields. |
| pkg-py/CHANGELOG.md | Documents new metadata support. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
# Conflicts: # pkg-py/tests/test_datasource.py # pkg-py/tests/test_tools.py
Addresses Copilot review on #331: a values map on a numeric column (e.g. {1: Low, 2: High}) was ignored because values handling lived in the text-column branch, so the LLM lost the coding meanings. Spec values now replace live stats/categorical inference for any column kind, matching pkg-py.
# Conflicts: # pkg-py/tests/test_data_dict.py # pkg-r/tests/testthat/test-querychat_tools.R
Addresses Copilot review on #331: a numeric/date column with values but no range no longer uses live stats, so requesting them scanned remote tables for nothing. When all columns are documented, the aggregate query is now skipped entirely.
The data dictionary guide now covers table label/constraints, column label/time_zone/examples/display, and relationship aliases/conflicts. Changelog entries lead with what users can do rather than rendering internals.
# Conflicts: # pkg-py/docs/context.qmd # pkg-r/R/DBISource.R # pkg-r/vignettes/context.Rmd
Addresses Copilot review on #331: module is already private, so helper names don't need a leading underscore per project convention.
| Each entry under `tables` describes one table. Its `name` must match the table name you pass to `QueryChat` or `.add_table()`. | ||
|
|
||
| - **`description`**: What this table represents (one sentence is usually enough). | ||
| - **`label`**: Optional display title, shown alongside the table in the schema view. |
| Each entry under `tables` describes one table. Its `name` must match the table name you pass to `QueryChat$new()` or `$add_table()`. | ||
|
|
||
| - **`description`**: What this table represents (one sentence is usually enough). | ||
| - **`label`**: Optional display title, shown alongside the table in the schema view. |
# Conflicts: # pkg-r/NEWS.md
This branch has not been deployed
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.
Stacked on #330 (phase 1, data-dict spec alignment) — review that PR first. #316 follow-up.
Summary
Wires the data-dict spec's newer descriptive fields into querychat's prompt and schema channels, in both packages:
constraints(assertion objects/barewords) are rendered in theget_schematool response as aTable constraints:section.labelandtime_zoneappear asLabel:/Time zone:annotations in the LLM schema text;labelalso passes through to the system-prompt data-dict block (both packages).examplesare shown in the schema text, capped at 5 with a… (N more)marker.valueslabels are rendered as a trailingLabels: M=Male, F=Femaleannotation, keepingCategorical values:pure ('M','F') so the LLM writes SQL against real values.display: restrictedcolumns stay visible to the LLM (annotated[restricted: do not display in user-facing output], examples withheld) but are hidden from the user-facing schema card; queries are not blocked (soft restriction).aliases/conflictsparse in pkg-py and pass through to the system-prompt data-dict YAML block in both packages.pkg-py carries the new fields on
ColumnMeta(label,time_zone,display,examples,value_labels) fromDataDict.get_table_schema()throughformat_schema()and_schema_table(). pkg-r mirrors this with loose-list pass-throughs inbuild_column_details_impl(),format_schema_from_details()(which now takes an optionaltable_specso all schema-text paths include table constraints), andschema_table().Verification
test_new_field_parsing,test_get_table_schema_new_column_fields,test_to_prompt_dict_new_pass_throughs,test_format_schema_new_annotations,test_schema_card_hides_restricted_columns,test_get_schema_includes_table_constraints(py); "renders new dict fields and hides restricted columns from the card" (R).