feat(fields): add verbosity=slim to list-available-fields#535
Conversation
|
Thanks for the contribution! Unfortunately we can't verify the commit author(s): Nawar Tamawi <n***@s***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, refresh the status of this Pull Request. |
14fd1b4 to
4182d34
Compare
|
Review — posted by MattGPT (automation on Matt Filbert's behalf, approved by Matt). P2 —
Full mode is unaffected: its header does the same hoist for display but still returns the complete per-field array — so the earlier verification of the hoist as "not lossy" was correct for that path; this only bites slim. Suggested fix: keep |
…in (#537) Byte-sync of lockstep classify.ts from a2td claude/w2c-propose-identity (8181dd5): LlmProposeInput.fields gains optional datasource/column_ref, emitted only when the summary has >1 datasource or duplicate display names; single-ds payloads byte-identical. Manifest regenerated via check-lockstep --update; both repos' manifests now carry identical hash 87193686 for the classify twin (CLS-003 procedure). INERT until plumbed on this side (same pattern as the #511 semanticRole landing): tmcp's propose-instruction/prewarm surfaces don't yet emit the qualified identities; that plumbing rides the #535 fields-tool coordination per the port-debt ledger. Validation firsthand: full suite 4,346 green (298 files), tsc clean, check-lockstep 6/6. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…#217+#222 port, seam-1 packet A) (#539) * feat(desktop): exact column_ref first-class + shared ref helpers (a2td #217+#222 port, seam-1 packet A) Port-wave packet A from the content audit. #222 half: exported COLUMN_REF_REGEX + parseDatasourceQualifiedColumnRef / parseColumnInstanceRef / parseCanonicalColumnRef / formatCanonicalColumnRef in field-resolver (same API as a2td), private regexes in fields.ts / explicit-bind.ts replaced; preserved divergence (explicit-bind accepts bare [deriv:Base:sfx], fields require qualified); colon-tolerant instance parsing (fixes [Profit:Ratio] -> [Profit] mis-parse, caught by minion red run). #217 half: resolveField and proposal-binding resolveInSummary recognize exact canonical column_ref FIRST — hit resolves exact, ref-SHAPED miss fails closed (never falls to fuzzy); datasource selector matches internal name or UNIQUE caption (ambiguous caption refuses with guidance, Parameters excluded). Gate 5b closure untouched; listAvailableFields untouched (#535's file). Version 2.25.4 assumes #538 (2.25.3) merges first — resolve upward on conflict. Cursor-minion port (gpt-5.5-high), orchestrator-adjudicated: full suite 4,362 green firsthand, all 3 wrong-bind invariant suites green (210), tsc clean, lockstep 6/6, eslint clean (4 fixable errors fixed at adjudication). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style: prettier fix on port test files Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ource selection (a2td #220+#219 port, seam-1 packet B) (#540) * feat(desktop): exact column_ref first-class + shared ref helpers (a2td #217+#222 port, seam-1 packet A) Port-wave packet A from the content audit. #222 half: exported COLUMN_REF_REGEX + parseDatasourceQualifiedColumnRef / parseColumnInstanceRef / parseCanonicalColumnRef / formatCanonicalColumnRef in field-resolver (same API as a2td), private regexes in fields.ts / explicit-bind.ts replaced; preserved divergence (explicit-bind accepts bare [deriv:Base:sfx], fields require qualified); colon-tolerant instance parsing (fixes [Profit:Ratio] -> [Profit] mis-parse, caught by minion red run). #217 half: resolveField and proposal-binding resolveInSummary recognize exact canonical column_ref FIRST — hit resolves exact, ref-SHAPED miss fails closed (never falls to fuzzy); datasource selector matches internal name or UNIQUE caption (ambiguous caption refuses with guidance, Parameters excluded). Gate 5b closure untouched; listAvailableFields untouched (#535's file). Version 2.25.4 assumes #538 (2.25.3) merges first — resolve upward on conflict. Cursor-minion port (gpt-5.5-high), orchestrator-adjudicated: full suite 4,362 green firsthand, all 3 wrong-bind invariant suites green (210), tsc clean, lockstep 6/6, eslint clean (4 fixable errors fixed at adjudication). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(desktop): planner datasource-qualified fields + apply-side datasource selection (a2td #220+#219 port, seam-1 packet B) Port-wave packet B, stacked on packet A (#539). #220 half: planner fields accept exact column_ref OR {query, datasource?}; resolution cache keyed on (query, selector); task specs carry the resolved common datasource; ambiguous AND not_found both block the plan (closes the audit-verified hole where not_found-only still planned with a warning). #219 half: buildAndApplyWorksheet derives the rewrite datasource from explicitBind.datasource — the caption/first-workbook-datasource regex grab is DELETED; no-manifest passthrough infers a single datasource from the provided refs via the shared parser and FAILS CLOSED on mixed refs with a prescriptive block; injectTemplate blocks caller DATASOURCE that disagrees with the resolved mapping datasource. Two characterization pins deliberately re-baselined (caption-first -> common-ref datasource), each with a comment naming the new invariant. Zero contact with the receipt-formatting area (#538's file overlap — verified no conflict). Cursor-minion port (gpt-5.5-high), orchestrator-adjudicated: full suite 4,368 green firsthand, invariant suites 210 green, tsc clean, lockstep 6/6, eslint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…tentUrl Slim previously hoisted fields[0].datasource and dropped per-field datasource, misattributing every field past the first in multi-datasource workbooks and erasing the disambiguator for same-caption fields (review on tableau#535). Slim now always returns { count, datasources: [{ datasource, contentUrl?, fields }] } — grouped by datasource so the name is carried once per group (small), and correct for 2+ datasources. contentUrl (from repository-location) is surfaced per published datasource as the input resolve-datasource-luid needs; omitted for embedded ones. Single datasource is one group, so callers parse one consistent shape.
|
Two things before the next round, both verified against current 1. Rebase needed — the branch predates the live-session contract. Current 2. Run the byte-ratchet tests before merge. The multi-datasource slim grouping itself looks right, and dropping (Posted by MattGPT, Matt's review automation; Matt reviewed the findings.) |
list-available-fields returns the field catalog twice (a human-readable ASCII
table plus a full per-field metadata array incl. column_ref) — ~68KB for a wide
datasource (155 fields), which overflows the host's inline-output cap and gets
truncated to a preview the caller can't fully re-read.
Add an optional verbosity param:
- 'full' (default): unchanged — table + full metadata + column_ref. Backward
compatible for authoring callers that place fields on shelves.
- 'slim': a compact JSON array of { caption, role, datatype } with datasource
hoisted to the top level, no ASCII table, no authoring metadata. Small enough
to land inline, for reasoning over fields to pick measures/dimensions (e.g.
generate-insight-cards). Resolve the exact column_ref via resolve-field.
Adds slim-mode tests.
…tentUrl Slim previously hoisted fields[0].datasource and dropped per-field datasource, misattributing every field past the first in multi-datasource workbooks and erasing the disambiguator for same-caption fields (review on tableau#535). Slim now always returns { count, datasources: [{ datasource, contentUrl?, fields }] } — grouped by datasource so the name is carried once per group (small), and correct for 2+ datasources. contentUrl (from repository-location) is surfaced per published datasource as the input resolve-datasource-luid needs; omitted for embedded ones. Single datasource is one group, so callers parse one consistent shape.
…a budget The grouped-shape example inlined in the verbosity param description pushed list-available-fields to 1229 bytes (over the 1200-byte per-tool cap) and the total schema 18 bytes over 46000. Trim to a concise description; the grouped shape is self-describing in the response.
|
Non-blocking follow-up: the slim projection tests cover mocked |
tonythebest
left a comment
There was a problem hiding this comment.
The multi-datasource issue is fixed, the grouped slim contract is sound, and CI is green. Approved; please rebase on feature/desktop and rerun CI before merge.
Addresses review: no existing slim test exercised the live-session path. Adds a verbosity=slim test with a session and no workbookFile, asserting the grouped slim shape and that the file cache is never read.
Addresses review: keep the desktop tools/list surface under the 46k auto-deferral cliff (server.desktop.test.ts). Trims the verbosity param text to the load-bearing facts rather than raising the cap.
b48cc36 to
ea5b4a7
Compare
…tentUrl Slim previously hoisted fields[0].datasource and dropped per-field datasource, misattributing every field past the first in multi-datasource workbooks and erasing the disambiguator for same-caption fields (review on tableau#535). Slim now always returns { count, datasources: [{ datasource, contentUrl?, fields }] } — grouped by datasource so the name is carried once per group (small), and correct for 2+ datasources. contentUrl (from repository-location) is surfaced per published datasource as the input resolve-datasource-luid needs; omitted for embedded ones. Single datasource is one group, so callers parse one consistent shape.
ea5b4a7 to
60b15c3
Compare
|
good points @tonythebest both concerns addressed. |
… default parity Takeover fixes on the verbosity=slim feature: slim output now carries localName, columnInstanceName, derivation, type, and typePivot — without them a consumer of slim output cannot construct the compound column reference the authoring tools require (the tool's whole purpose). Already- aggregated calc fields covered in slim (usr: + isAggregated). verbosity is zod-enum validated; absent verbosity is pinned byte-for-byte to full. Descriptions tightened for the combined-lean budget. Co-Authored-By: Claude <noreply@anthropic.com>
…lds kept, 46k cliff assertion retired Conflicts: package.json keeps this branch's 2.31.0 (lock regenerated to match — also heals feature/desktop's own 2.30.0-json/2.29.2-lock drift on merge); listAvailableFields takes the feature/desktop side wholesale (the #535 slim verbosity feature, already terse). The merged surface put the combined-lean tools/list 149 bytes over the 46k cliff — the assertion is RETIRED per owner decision (2026-07-21): profile-based serving is the operative constraint, the dynamic profile sits far under any deferral limit, and full route coverage outranks squeezing full-surface prose. Unused test helpers pruned with it. Validated: tsc + eslint clean, full suite 4988 passed / 1 skipped. Co-Authored-By: Claude <noreply@anthropic.com>
Adds an optional
verbosity: 'slim' | 'full'param tolist-available-fields.full(default) — unchanged: human-readable table + full per-field metadata incl.column_ref.slim— no table; fields grouped by datasource:{ caption, role, datatype }. The datasource name is carried once per group (not repeated per field), andcontentUrlis included per published datasource (the inputresolve-datasource-luidneeds; omitted for embedded ones). A single-datasource workbook is one group, so callers always parse one shape.Why: the default response returns the field catalog twice (table + full metadata) — ~68KB for a wide datasource (~155 fields), which overflows the inline-output cap and gets truncated.
slimstays well under the cap for callers that only need to reason over field names/roles/types. Get the exactcolumn_refviaresolve-field.Verbosity param rather than a new tool to avoid the
tools/listschema budget.Grouping (addresses @mattcfilbert's review): slim originally hoisted
fields[0].datasourceand dropped per-field datasource — misattributing every field past the first in multi-datasource workbooks and erasing the disambiguator for same-caption fields. Slim now always groups by datasource, which is correct for 2+ datasources and keeps the payload small (datasource string once per group, not per field). Added multi-datasource + duplicate-caption + embedded (nocontentUrl) tests;fullpath unchanged.