feat(pframes): add keysOnly column option to tableBuilder#1702
Conversation
addColumn/addColumns accept { keysOnly: true }: the column participates in
the join (its key space is unioned into the output rows and any new axes are
surfaced) but its value column is not written to the exported file.
Implemented by skipping the value selector in buildExportSelectors; the flag
is serialized into the build-table template input only when true, so existing
(non-keys-only) usage produces identical resources and dedup is preserved.
🦋 Changeset detectedLatest commit: 35db48a The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a keysOnly option to addColumn and addColumns in tableBuilder, allowing columns to participate in joins and expand the key space (or surface new axes) without writing their value columns to the exported file. The changes span table-builder.lib.tengo, build-table.tpl.tengo, export-util.lib.tengo, and include new tests in table-builder.test.ts. Feedback on the changes includes addressing a potential runtime panic in Tengo when evaluating the undefined value of column.keysOnly in an if statement, and implementing typo guards for the options maps in addColumn and addColumns to prevent misspelled options from being silently ignored.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if column.keysOnly { | ||
| continue | ||
| } |
There was a problem hiding this comment.
In Tengo, using a non-boolean value (such as undefined) as an if condition will cause a runtime panic (non-boolean condition in if statement). Since primary columns in rawColumns do not have the keysOnly property set, column.keysOnly will resolve to undefined and crash the execution.
Please change the condition to column.keysOnly == true to safely handle undefined values.
if column.keysOnly == true {
continue
}
| if len(opts) > 0 && ll.isMap(opts[0]) { | ||
| header = opts[0].header | ||
| keysOnly = opts[0].keysOnly == true | ||
| } |
There was a problem hiding this comment.
According to the project's general rules, we should implement a typo guard when accepting options maps to validate and reject unknown keys. This prevents misspelled options from being silently ignored.
Please add a typo guard to validate the keys of the opts[0] map.
if len(opts) > 0 && ll.isMap(opts[0]) {
for k, _ in opts[0] {
ll.assert(k == "header" || k == "keysOnly", "unknown option %q", k)
}
header = opts[0].header
keysOnly = opts[0].keysOnly == true
}
References
- When accepting options maps (e.g., opts), implement a typo guard that validates and rejects unknown keys to prevent misspelled options from silently being ignored and falling back to defaults.
| if len(opts) > 0 && ll.isMap(opts[0]) { | ||
| headerPrefix = opts[0].headerPrefix | ||
| headerSuffix = opts[0].headerSuffix | ||
| keysOnly = opts[0].keysOnly == true | ||
| } |
There was a problem hiding this comment.
According to the project's general rules, we should implement a typo guard when accepting options maps to validate and reject unknown keys. This prevents misspelled options from being silently ignored.
Please add a typo guard to validate the keys of the opts[0] map.
if len(opts) > 0 && ll.isMap(opts[0]) {
for k, _ in opts[0] {
ll.assert(k == "headerPrefix" || k == "headerSuffix" || k == "keysOnly", "unknown option %q", k)
}
headerPrefix = opts[0].headerPrefix
headerSuffix = opts[0].headerSuffix
keysOnly = opts[0].keysOnly == true
}
References
- When accepting options maps (e.g., opts), implement a typo guard that validates and rejects unknown keys to prevent misspelled options from silently being ignored and falling back to defaults.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| expect(lines.length).toBe(4); // header + 3 data rows | ||
| }, | ||
| ); | ||
|
|
||
| // keys-only column: an enrichment whose key space is unioned into the output | ||
| // (its keys appear as rows) but whose value column is NOT written to the file. | ||
| // `extra` carries id rows A, C, D — primary carries A, B, C. The full-join | ||
| // surfaces D (a key only the keys-only column knows about) while its value | ||
| // stays out of the table. | ||
| const extraCsv = dedent` | ||
| id,extra | ||
| A,111 | ||
| C,333 | ||
| D,444 | ||
| `; | ||
| const extraSpec = specWithLong("extra", "pl7.app/extra", "Extra"); | ||
|
|
||
| tplTest.concurrent( | ||
| "tableBuilder: keysOnly column expands key space without emitting a value column", | ||
| async ({ helper, expect, driverKit }) => { | ||
| const content = await runEphemeralBuilder(helper, driverKit, { | ||
| format: "tsv", | ||
| imports: { | ||
| main: { csv: primaryCsv, spec: primarySpec }, | ||
| extra: { csv: extraCsv, spec: extraSpec }, | ||
| }, | ||
| primaries: [{ name: "main", sourceImport: "main" }], | ||
| columns: [{ mode: "single", sourceImport: "extra", opts: { keysOnly: true } }], | ||
| }); | ||
| const lines = content.trim().split("\n"); | ||
|
|
||
| // Header has ONLY the primary's axis + value column — the keys-only | ||
| // column's value ("Extra") is absent. This is the core assertion. | ||
| expect(lines[0]).toBe("ID\tScore"); | ||
|
|
||
| // Union of keys {A,B,C} ∪ {A,C,D} = {A,B,C,D}. D comes solely from the | ||
| // keys-only column, proving its key space was unioned into the output. | ||
| // (Assert on the key column rather than full rows: D's Score cell is | ||
| // empty, and content.trim() would strip that trailing tab.) | ||
| const dataLines = lines.slice(1); | ||
| const ids = dataLines.map((l) => l.split("\t")[0]).sort(); | ||
| expect(ids).toEqual(["A", "B", "C", "D"]); | ||
|
|
||
| // D carries no Score value (empty cell) — it contributed only its key. | ||
| const dRow = dataLines.find((l) => l.split("\t")[0] === "D")!; | ||
| expect(dRow.split("\t")[1] ?? "").toBe(""); | ||
| // Existing keys keep their primary value. | ||
| expect(dataLines.find((l) => l.split("\t")[0] === "A")).toBe("A\t10"); | ||
| }, | ||
| ); | ||
|
|
||
| tplTest.concurrent( | ||
| "tableBuilder: same column without keysOnly emits its value column (contrast)", | ||
| async ({ helper, expect, driverKit }) => { | ||
| const content = await runEphemeralBuilder(helper, driverKit, { | ||
| format: "tsv", | ||
| imports: { | ||
| main: { csv: primaryCsv, spec: primarySpec }, | ||
| extra: { csv: extraCsv, spec: extraSpec }, | ||
| }, | ||
| primaries: [{ name: "main", sourceImport: "main" }], | ||
| columns: [{ mode: "single", sourceImport: "extra", opts: { header: "Extra" } }], | ||
| }); | ||
| const lines = content.trim().split("\n"); | ||
|
|
||
| // Without keysOnly the value column IS written, proving the flag is what | ||
| // suppresses it (not some unrelated join behaviour). | ||
| expect(lines[0]).toBe("ID\tScore\tExtra"); | ||
| expect(lines.slice(1).sort()).toEqual(["A\t10\t111", "B\t20\t", "C\t30\t333", "D\t\t444"]); | ||
| }, | ||
| ); | ||
|
|
||
| // keys-only column that introduces a NEW axis. The primary is keyed by `id` | ||
| // only; the keys-only `byRegion` column is keyed by (id, region). The full-join | ||
| // surfaces `region` as a new output column (its keys expand the rows) while the | ||
| // column's own value is NOT written. This guards the "new axes are surfaced" | ||
| // half of the feature — distinct from the shared-axis case above, this test | ||
| // fails if the axis-collection loop skipped keys-only columns. | ||
| const byRegionCsv = dedent` | ||
| id,region,rv | ||
| A,north,1 | ||
| A,south,2 | ||
| B,north,3 | ||
| `; | ||
| const byRegionSpec = { | ||
| axes: [ | ||
| { | ||
| column: "id", | ||
| spec: { | ||
| name: "pl7.app/id", | ||
| type: "String", | ||
| annotations: { [Annotation.Label]: "ID" } satisfies Annotation, | ||
| }, | ||
| }, | ||
| { | ||
| column: "region", | ||
| spec: { | ||
| name: "pl7.app/region", | ||
| type: "String", | ||
| annotations: { [Annotation.Label]: "Region" } satisfies Annotation, | ||
| }, | ||
| }, | ||
| ], | ||
| columns: [ | ||
| { | ||
| column: "rv", | ||
| id: "rv", | ||
| spec: { | ||
| valueType: "Long", | ||
| name: "pl7.app/region-value", | ||
| annotations: { [Annotation.Label]: "RegionValue" } satisfies Annotation, | ||
| }, | ||
| }, | ||
| ], | ||
| storageFormat: "Json", | ||
| partitionKeyLength: 0, | ||
| }; | ||
|
|
||
| tplTest.concurrent( | ||
| "tableBuilder: keysOnly column surfaces a new axis without emitting its value", | ||
| async ({ helper, expect, driverKit }) => { | ||
| const content = await runEphemeralBuilder(helper, driverKit, { | ||
| format: "tsv", | ||
| imports: { | ||
| main: { csv: primaryCsv, spec: primarySpec }, | ||
| byRegion: { csv: byRegionCsv, spec: byRegionSpec }, | ||
| }, | ||
| primaries: [{ name: "main", sourceImport: "main" }], | ||
| columns: [{ mode: "single", sourceImport: "byRegion", opts: { keysOnly: true } }], | ||
| }); | ||
| const lines = content.trim().split("\n"); | ||
|
|
||
| // New axis "Region" is surfaced (between the primary axis and value), but | ||
| // the keys-only column's own value ("RegionValue") is NOT in the header. | ||
| expect(lines[0]).toBe("ID\tRegion\tScore"); | ||
|
|
||
| // Full-join over the (id, region) key space: A expands to north+south | ||
| // (Score broadcast = 10), B to north (20), C has no region row so its | ||
| // Region cell is empty (Score 30). Every row keeps its primary Score. | ||
| expect(lines.slice(1).sort()).toEqual([ | ||
| "A\tnorth\t10", | ||
| "A\tsouth\t10", | ||
| "B\tnorth\t20", | ||
| "C\t\t30", | ||
| ]); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
addColumns multi-match path with keysOnly
All three new tests use mode: "single" (i.e. addColumn), so the addColumns multi-match code path — where keysOnly is threaded through a multiResult expansion in build-table.tpl.tengo — has no integration test coverage. If something regressed in the if !is_undefined(c.multiResult) branch (e.g. the flag silently dropped for one of the sub-entries), the existing tests would not catch it. A fourth test exercising addColumns with a multi-match query spec and keysOnly: true would close this gap.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/workflow-tengo/src/pframes/table-builder.test.ts
Line: 408-554
Comment:
**Missing test coverage for `addColumns` multi-match path with `keysOnly`**
All three new tests use `mode: "single"` (i.e. `addColumn`), so the `addColumns` multi-match code path — where `keysOnly` is threaded through a `multiResult` expansion in `build-table.tpl.tengo` — has no integration test coverage. If something regressed in the `if !is_undefined(c.multiResult)` branch (e.g. the flag silently dropped for one of the sub-entries), the existing tests would not catch it. A fourth test exercising `addColumns` with a multi-match query spec and `keysOnly: true` would close this gap.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1702 +/- ##
===========================================
+ Coverage 44.56% 55.01% +10.44%
===========================================
Files 43 310 +267
Lines 2540 17426 +14886
Branches 663 3802 +3139
===========================================
+ Hits 1132 9587 +8455
- Misses 1225 6607 +5382
- Partials 183 1232 +1049 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
What
tableBuilder.addColumn/addColumnsaccept{ keysOnly: true }. A keys-only column participates in the join — its key space is unioned into the output rows and any new axes are surfaced — but its value column is not written to the exported file. It's the full-join sibling of the existingfilter(which inner-joins and is also not rendered).Why
Lets a table pull in the key space of a column (expand rows, surface an axis) without emitting that column's values.
How
table-builder.lib.tengo— threadkeysOnlythroughaddColumn/addColumnsopts. Serialized into the build-table template input only when true, so existing (non-keys-only) usage produces identical resources → dedup preserved.build-table.tpl.tengo— carry the flag into enrichment /rawColumnsentries.export-util.lib.tengo— skip the value selector and its label for keys-only columns; their axes are still collected (so new axes appear in the output).Tests
3 integration tests, run against a live backend:
(id, region)keys-only column addsregionas an output column, value dropped),Greptile Summary
This PR adds a
keysOnlyoption totableBuilder.addColumn/addColumns. A keys-only column joins into the full-join tree (unioning its key space and surfacing any new axes) but emits no value column in the exported file.table-builder.lib.tengo: threadskeysOnlyfrom user opts through internal column state and the template input; the flag is omitted from the serialized resource whenfalse, preserving resource dedup for all existing callers.build-table.tpl.tengo/export-util.lib.tengo: carries the flag into enrichment entries andrawColumns; the export selector loop skips the value selector andregisterLabelcall for keys-only columns while the axis-collection loop still visits them (so new axes are surfaced). The guardc.keysOnly == truehandles the missing-field case safely via Tengo's falsyundefined.addColumnsmulti-match code path withkeysOnlyis not exercised.Confidence Score: 4/5
Safe to merge; all changed code paths are correct and backwards-compatible with existing callers.
The logic is implemented cleanly and handles all documented cases. The only gap is that the addColumns multi-match branch (multiResult expansion) with keysOnly: true has no integration test, so a regression there would be invisible to CI.
The test file — specifically the absence of a multi-match addColumns + keysOnly test — is the only area worth a second look before merging.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[addColumn / addColumns with keysOnly opt] --> B{keysOnly == true?} B -- yes --> C[column stored with keysOnly:true] B -- no --> D[column stored with keysOnly:false] C --> E{build: serialize to columnsMap} D --> E E -- keysOnly=true --> F[entry.keysOnly = true emitted] E -- keysOnly=false --> G[keysOnly field omitted, dedup preserved] F --> H[build-table.tpl: keysOnly := c.keysOnly == true] G --> H H --> I[enrichment entry carries keysOnly flag] I --> J[rawColumns entry carries keysOnly flag] J --> K[buildExportSelectors] K --> L[Axis loop: ALL columns incl. keys-only contribute axes] K --> M{column.keysOnly?} M -- yes --> N[skip registerLabel + value selector] M -- no --> O[emit value selector with label] L --> P[sortSelectors + axis columnSelectors] O --> P N --> P%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[addColumn / addColumns with keysOnly opt] --> B{keysOnly == true?} B -- yes --> C[column stored with keysOnly:true] B -- no --> D[column stored with keysOnly:false] C --> E{build: serialize to columnsMap} D --> E E -- keysOnly=true --> F[entry.keysOnly = true emitted] E -- keysOnly=false --> G[keysOnly field omitted, dedup preserved] F --> H[build-table.tpl: keysOnly := c.keysOnly == true] G --> H H --> I[enrichment entry carries keysOnly flag] I --> J[rawColumns entry carries keysOnly flag] J --> K[buildExportSelectors] K --> L[Axis loop: ALL columns incl. keys-only contribute axes] K --> M{column.keysOnly?} M -- yes --> N[skip registerLabel + value selector] M -- no --> O[emit value selector with label] L --> P[sortSelectors + axis columnSelectors] O --> P N --> PPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(pframes): add keysOnly column optio..." | Re-trigger Greptile