fix: separate pframe column id key and counter to prevent id collisions

[PoslavskySV]

Before

Export column ids concatenated the map key and a monotonic counter with no separator (k + string(i)), so two distinct (key, counter) pairs could collapse to the same id — e.g. sequence_1 + 18 and sequence_11 + 8 both rendered to sequence_118. The SDK pframe builder correctly rejects the duplicate id, which surfaced in production as:

tengo template error: assertion error: condition failed: field "sequence_….spec" is already set

It fired intermittently: the collision needs key names that end in digits and the counter to reach ≥ 10 for the boundary to become ambiguous, so whether it triggered depended on dataset shape / column count.

After

A "_" separator makes each id unambiguous — sequence_1_18 vs sequence_11_8. string(i) is pure digits, so the last underscore always splits the id back into a unique (key, counter) pair; no two pairs can collide.

How

k + string(i) → k + "_" + string(i) at both pframe-builder sites in workflow/src/outputs.lib.tengo (2 hunks, nothing else changed). Sibling fixed-prefix sites (single "…" + string(i) calls) are intentionally left as-is — the added underscore also isolates the k + "_" + … ids from those fixed-prefix ids.

Caveat — internal column ids change

This changes internal pframe column ids (sequence_118 → sequence_1_18). Verified while making the change that column selection is spec-based (SUniversalPColumnId), not the builder id, and that these dynamically-generated ids can't be referenced literally by any static consumer or test. The one thing to confirm at merge: that no persisted per-project view state (table column order/visibility, plot axis defaults) keys off the literal builder id — if it did, existing saved projects would reset that view state to defaults on next run (cosmetic; no data loss, no block failure).

Greptile Summary

This PR fixes an intermittent production crash caused by pFrame column id collisions in workflow/src/outputs.lib.tengo. When map keys ended in digits, concatenating the counter directly (k + string(i)) could merge two distinct (key, counter) pairs into the same id string, which the SDK pframe builder rejected with an assertion error.

• Both pFrameBuilder sites in buildOutputPFrames now separate key and counter with an underscore: k + \"_\" + string(i), eliminating the collision.
• Sibling fixed-prefix sites (addCols helper, parquet table headers) are correctly left unchanged.
• Key terms: pFrame column id (changed from k + string(i) to k + \"_\" + string(i)), pFrameBuilder (SDK construct enforcing id uniqueness), resultsPf (UI results pFrame), epf/exportPf (export pFrame), SUniversalPColumnId (spec-based column selection, unaffected).

Confidence Score: 5/5

Safe to merge — the change is a two-line, targeted fix to a confirmed production crash with no side effects on data correctness or block logic.

The fix is minimal and correct: adding an underscore separator between the map key and the monotonic counter eliminates the id-collision path. Both changed call sites are symmetric and consistent. The only consequence is that internal builder ids change, which could reset persisted per-project view state — already identified and accepted by the PR author as cosmetic with no data loss.

No files require special attention; the single changed file touches only two lines and the fix logic is straightforward.

Important Files Changed

Filename	Overview
workflow/src/outputs.lib.tengo	Two pFrameBuilder add call sites changed from k + string(i) to k + "_" + string(i) to prevent column-id collisions when map keys end in digits; fix is minimal, correct, and well-reasoned.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant buildOutputPFrames
    participant pFrameBuilder as pFrameBuilder (SDK)

    Caller->>buildOutputPFrames: importedResults, toTargetPf, targetMatchCountPf, refMatchCountPf

    Note over buildOutputPFrames: Loop 1 - resultsPf
    loop for k, v in importedResults
        buildOutputPFrames->>pFrameBuilder: add(k + underscore + string(i), v.spec, v.data)
    end
    pFrameBuilder-->>buildOutputPFrames: resultsPf

    Note over buildOutputPFrames: Loop 2 - epf
    loop for pf in toTargetPf, targetMatchCountPf, refMatchCountPf
        loop for k, v in pf
            buildOutputPFrames->>pFrameBuilder: add(k + underscore + string(i), v.spec, v.data)
        end
    end
    pFrameBuilder-->>buildOutputPFrames: epf

    buildOutputPFrames-->>Caller: resultsPf and exportPf

Loading

%%{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"}}}%%
sequenceDiagram
    participant Caller
    participant buildOutputPFrames
    participant pFrameBuilder as pFrameBuilder (SDK)

    Caller->>buildOutputPFrames: importedResults, toTargetPf, targetMatchCountPf, refMatchCountPf

    Note over buildOutputPFrames: Loop 1 - resultsPf
    loop for k, v in importedResults
        buildOutputPFrames->>pFrameBuilder: add(k + underscore + string(i), v.spec, v.data)
    end
    pFrameBuilder-->>buildOutputPFrames: resultsPf

    Note over buildOutputPFrames: Loop 2 - epf
    loop for pf in toTargetPf, targetMatchCountPf, refMatchCountPf
        loop for k, v in pf
            buildOutputPFrames->>pFrameBuilder: add(k + underscore + string(i), v.spec, v.data)
        end
    end
    pFrameBuilder-->>buildOutputPFrames: epf

    buildOutputPFrames-->>Caller: resultsPf and exportPf

Loading

_{Reviews (1): Last reviewed commit: "fix: separate pframe column id key and c..." | Re-trigger Greptile}

Context used:

• Context used - Terms is a types in codebase. Provide the list of ... (source)

[gemini-code-assist]

Code Review

This pull request updates workflow/src/outputs.lib.tengo to add an underscore separator before a sequential index when building output frames. The reviewer notes that map iteration in Tengo is non-deterministic, meaning that appending a sequential counter during map iteration results in unstable column IDs across runs. To resolve this, the reviewer suggests using the unique keys directly for importedResults and applying deterministic suffixes for the other maps to ensure stable, reproducible column IDs.

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.

[gemini-code-assist]

In Tengo (which is implemented in Go), map iteration order is randomized and non-deterministic. Because importedResults is a map, iterating over it with for k, v in importedResults and appending a sequential counter i (i.e., k + "_" + string(i)) results in non-deterministic column IDs across different runs of the workflow.

Since the keys k in importedResults are already guaranteed to be unique (e.g., targetSeq0, targetMatchCount, etc.), we can simply use k directly without any counter suffix. This ensures stable, deterministic column IDs.

	for k, v in importedResults {
		resultsPf.add(k, v.spec, v.data)
	}

[gemini-code-assist]

Similarly, iterating over the maps toTargetPf, targetMatchCountPf, and refMatchCountPf using a sequential counter i introduces non-determinism in the exported column IDs because map iteration order is randomized.

To make the column IDs deterministic and descriptive, we can add the columns from each map individually with a deterministic suffix (e.g., _toTarget, _target, _ref). This completely avoids ID collisions while keeping the output stable across runs.

	for k, v in toTargetPf {
		epf.add(k + "_toTarget", v.spec, v.data)
	}
	for k, v in targetMatchCountPf {
		epf.add(k + "_target", v.spec, v.data)
	}
	for k, v in refMatchCountPf {
		epf.add(k + "_ref", v.spec, v.data)
	}