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/main.tpl.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 ID-collision bug in the Tengo workflow template where two different (key, counter) pairs could produce the same pframe column ID. For example, key sequence_1 with counter 18 and key sequence_11 with counter 8 both yielded the string sequence_118, causing the pframe builder to reject the duplicate and surface a hard error at runtime.

The fix adds a "_" separator between the map key and the monotonic counter at two pFrameBuilder.add call sites — one building the MSA pframe (msaPf) and one building the export/plot pframe (epf).

• msaPf loop (line 565): changed k + string(i) → k + "_" + string(i) for columns merged from cloneToClusterLinkPf and distancesPf.
• epf loop (line 575): same change for the seven pframes merged into the main export pframe.
• The bubblePlotPfBuilder loop (line 553) and opf loop (line 469) use just k with no counter and are untouched by this PR, though they could be susceptible to a different form of key collision if the same key k appears across multiple source pframes.

Confidence Score: 5/5

Safe to merge. The two-line change is narrowly scoped, directly addresses a reproducible production failure, and the separator logic is sound.

The fix is minimal and correct: adding a underscore separator between the map key and the monotonic counter makes every generated column ID unambiguous within both builder loops. The only observation is a latent collision risk in the untouched bubblePlotPfBuilder loop, which was not introduced by this PR.

No files require special attention. It is worth verifying that the bubblePlotPfBuilder source pframes have disjoint key sets, but this is a pre-existing condition.

Important Files Changed

Filename	Overview
workflow/src/main.tpl.tengo	Two-character fix at two pFrameBuilder.add call sites resolves the production-visible column ID collision bug. Untouched pframe builder loops carry a latent key-collision risk not in scope for this PR.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph msaPf["msaPf (MSA pFrame)"]
        A[cloneToClusterLinkPf] -->|for k,v| M1["msaPf.add(k + '_' + string(i), ...)"]
        B[distancesPf] -->|for k,v| M1
        M1 --> M2[msaPf.build]
    end
    subgraph epf["epf (Export pFrame)"]
        C1[abundancesPf] -->|for k,v| E1["epf.add(k + '_' + string(i), ...)"]
        C2[cloneToClusterPf] -->|for k,v| E1
        C3[cloneToClusterLinkPf] -->|for k,v| E1
        C4[clusterToSeqPf] -->|for k,v| E1
        C5[abundancesPerClusterPf] -->|for k,v| E1
        C6[distancesPf] -->|for k,v| E1
        C7[clusterRadiusPf] -->|for k,v| E1
        E1 --> E2[epf.build]
    end
    subgraph bubble["bubblePlotPfBuilder (untouched)"]
        D1[abundancesTopPf] -->|for k,v| B1["add(k, ...)"]
        D2[clusterToSeqTopPf] -->|for k,v| B1
        D3[clusterRadiusTopPf] -->|for k,v| B1
        B1 --> B2[build]
    end
    style msaPf fill:#d4edda,stroke:#28a745
    style epf fill:#d4edda,stroke:#28a745
    style bubble fill:#fff3cd,stroke:#ffc107

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"}}}%%
flowchart TD
    subgraph msaPf["msaPf (MSA pFrame)"]
        A[cloneToClusterLinkPf] -->|for k,v| M1["msaPf.add(k + '_' + string(i), ...)"]
        B[distancesPf] -->|for k,v| M1
        M1 --> M2[msaPf.build]
    end
    subgraph epf["epf (Export pFrame)"]
        C1[abundancesPf] -->|for k,v| E1["epf.add(k + '_' + string(i), ...)"]
        C2[cloneToClusterPf] -->|for k,v| E1
        C3[cloneToClusterLinkPf] -->|for k,v| E1
        C4[clusterToSeqPf] -->|for k,v| E1
        C5[abundancesPerClusterPf] -->|for k,v| E1
        C6[distancesPf] -->|for k,v| E1
        C7[clusterRadiusPf] -->|for k,v| E1
        E1 --> E2[epf.build]
    end
    subgraph bubble["bubblePlotPfBuilder (untouched)"]
        D1[abundancesTopPf] -->|for k,v| B1["add(k, ...)"]
        D2[clusterToSeqTopPf] -->|for k,v| B1
        D3[clusterRadiusTopPf] -->|for k,v| B1
        B1 --> B2[build]
    end
    style msaPf fill:#d4edda,stroke:#28a745
    style epf fill:#d4edda,stroke:#28a745
    style bubble fill:#fff3cd,stroke:#ffc107

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
workflow/src/main.tpl.tengo:551-554
**`bubblePlotPfBuilder` has no separator against key collisions**

This loop uses the raw map key `k` with no counter. If the same key `k` exists in more than one of `abundancesTopPf`, `clusterToSeqTopPf`, or `clusterRadiusTopPf`, the pframe builder will receive a duplicate id and fail with the same type of error that this PR fixes on `msaPf`/`epf`. Worth confirming that the three `*Top` pframes are guaranteed to have disjoint key sets.

_{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 modifies workflow/src/main.tpl.tengo to add an underscore separator when generating column IDs (e.g., changing k + string(i) to k + "_" + string(i)). The reviewer identified a critical issue in both modified loops: because map iteration in Tengo is non-deterministic, pairing a sequential counter i with map keys during iteration results in unstable column IDs across different runs. This can break caching and cause flaky downstream behavior. The reviewer suggested extracting and sorting the map keys before iterating to ensure deterministic 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 based on Go), map iteration order is non-deterministic because Go map iteration is randomized. Since i is a sequential counter incremented during iteration, the resulting column IDs (e.g., k + "_" + string(i)) will be non-deterministic and change randomly across different runs of the same workflow.

This can break caching/memoization, cause flaky downstream behavior, and reset UI view states (like column order/visibility) on every run. To ensure determinism, we should extract and sort the keys of each pf before iterating over them.

		keys := []
		for k, _ in pf {
			keys = append(keys, k)
		}
		keys = slices.quickSort(keys)
		for k in keys {
			v := pf[k]
			msaPf.add(k + "_" + string(i), trace.inject(v.spec), v.data)
			i = i + 1
		}

[gemini-code-assist]

Similar to the previous loop, iterating over pf here is non-deterministic, which makes the generated column IDs in epf unstable across different runs. We should sort the keys of each pf before iterating to ensure deterministic column IDs.

		keys := []
		for k, _ in pf {
			keys = append(keys, k)
		}
		keys = slices.quickSort(keys)
		for k in keys {
			v := pf[k]
			epf.add(k + "_" + string(i), trace.inject(v.spec), v.data) // label, specs, data
			i = i + 1
		}