Julenmendieta/MILAB-6273_enableUpstream

[julenmendieta]

Greptile Summary

This PR enables upstream linker columns to flow into the histogram and scatter plot pickers by switching propertiesPfHandle from ctx.createPFrame to createPFrameForGraphs, adding a metaColumnPredicate to both graph pages, and introducing a dedicated columns.lib.tengo library that stamps pl7.app/isOutput: "true" on all sequence-properties output columns.

• workflow/src/columns.lib.tengo (new): Extracts all column-spec building from process.tpl.tengo into two public getters — forPropertiesPf (adds isOutput: "true") and forExport (score-only, blockId-stamped domain); trace.inject is removed from resultPframe but kept for exportPframe, preserving downstream provenance.
• model/src/index.ts: Adds isOutput !== "true" to the upstream linker column match predicate so the block's own output columns don't receive the "optional" visibility rule that was designed for upstream metadata columns.
• ui/src/pages/HistogramPage.vue & ScatterPage.vue: Tightens dataColumnPredicate to isOutput === "true" columns and introduces metaColumnPredicate for upstream grouping columns; both predicates include a pl7.app/trace check that is now dead code.

Confidence Score: 4/5

The core logic change is sound and the export pframe trace is correctly preserved; the two dead trace checks in the UI predicates are the only loose ends.

The column-library refactor and the isOutput annotation mechanism work correctly end-to-end. The metaColumnPredicate in both graph pages relies entirely on a trace check that can never match, so sequence-properties output columns will appear alongside upstream metadata in the meta/grouping picker. The stale comment adds confusion but no runtime impact.

ui/src/pages/HistogramPage.vue and ui/src/pages/ScatterPage.vue for the metaColumnPredicate dead-code issue; model/src/index.ts for the stale comment block.

Important Files Changed

Filename	Overview
model/src/index.ts	Switches propertiesPfHandle from ctx.createPFrame to createPFrameForGraphs, adds isOutput !== "true" guard to displayOptions.visibility, and adds isOutput annotation filter to upstream linker column match; stale comment contradicts the new call-site.
ui/src/pages/HistogramPage.vue	Tightens dataColumnPredicate to require isOutput=true and adds metaColumnPredicate; the pl7.app/trace guard in both predicates is dead code because trace injection was removed from propertiesPf columns.
ui/src/pages/ScatterPage.vue	Same dataColumnPredicate/metaColumnPredicate changes as HistogramPage.vue with the same dead trace guard.
workflow/src/columns.lib.tengo	New library extracting all column-spec building out of process.tpl.tengo; forPropertiesPf stamps pl7.app/isOutput: "true" on every column, forExport filters to isScore columns and stamps blockId on the domain. Clean refactor with proper deep-cloning of spec dicts.
workflow/src/process.tpl.tengo	Delegates column building to columns.lib.tengo, removes trace.inject from resultPframe.add (export frame still gets trace), and drops the inline exportColumns loop. Logic is clean and trace for export pframe is correctly preserved.
.changeset/fancy-readers-rest.md	Routine patch-level changeset entry for workflow, model, and UI packages.

Comments Outside Diff (2)

1. model/src/index.ts, line 157-163 (link)

   Stale comment contradicts the current implementation

   The block comment says "Use ctx.createPFrame instead of createPFrameForGraphs" and explains why createPFrameForGraphs was avoided (it would pull in exports.properties columns causing duplicates), but the code immediately below now calls createPFrameForGraphs. Any reader maintaining this file will be confused about which approach is intentional. The old justification no longer applies — the isOutput !== "true" guard and the upstreamMeta trace-filter together prevent the double-column issue — so the comment should be replaced with one that describes the new design.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: model/src/index.ts
   Line: 157-163
   
   Comment:
   **Stale comment contradicts the current implementation**
   
   The block comment says "Use `ctx.createPFrame` instead of `createPFrameForGraphs`" and explains why `createPFrameForGraphs` was avoided (it would pull in `exports.properties` columns causing duplicates), but the code immediately below now calls `createPFrameForGraphs`. Any reader maintaining this file will be confused about which approach is intentional. The old justification no longer applies — the `isOutput !== "true"` guard and the `upstreamMeta` trace-filter together prevent the double-column issue — so the comment should be replaced with one that describes the new design.
   
   How can I resolve this? If you propose a fix, please make it concise.

2. ui/src/pages/HistogramPage.vue, line 64-70 (link)

   pl7.app/trace checks are now dead code in both predicates

   trace.inject is no longer called when building resultPframe in process.tpl.tengo, so propertiesPf columns carry no pl7.app/trace annotation. Both predicates use !spec.annotations?.["pl7.app/trace"]?.includes("sequence-properties"), which evaluates to !undefined → true for every column and therefore has no filtering effect.

   For dataColumnPredicate this is harmless, because spec.annotations?.["pl7.app/isOutput"] === "true" is the real gate. For metaColumnPredicate the dead check is the only guard, so the predicate unconditionally returns true, allowing sequence-properties numeric output columns (Net Charge, GRAVY, etc.) to appear in the meta/grouping picker alongside upstream sample-level metadata. If the intent was to keep outputs out of the meta picker, the predicate should test isOutput instead of (or in addition to) trace. The same issue exists in ScatterPage.vue.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: ui/src/pages/HistogramPage.vue
   Line: 64-70
   
   Comment:
   **`pl7.app/trace` checks are now dead code in both predicates**
   
   `trace.inject` is no longer called when building `resultPframe` in `process.tpl.tengo`, so `propertiesPf` columns carry no `pl7.app/trace` annotation. Both predicates use `!spec.annotations?.["pl7.app/trace"]?.includes("sequence-properties")`, which evaluates to `!undefined` → `true` for every column and therefore has no filtering effect.
   
   For `dataColumnPredicate` this is harmless, because `spec.annotations?.["pl7.app/isOutput"] === "true"` is the real gate. For `metaColumnPredicate` the dead check is the *only* guard, so the predicate unconditionally returns `true`, allowing sequence-properties numeric output columns (Net Charge, GRAVY, etc.) to appear in the meta/grouping picker alongside upstream sample-level metadata. If the intent was to keep outputs out of the meta picker, the predicate should test `isOutput` instead of (or in addition to) `trace`. The same issue exists in `ScatterPage.vue`.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

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

---

### Issue 1 of 2
model/src/index.ts:157-163
**Stale comment contradicts the current implementation**

The block comment says "Use `ctx.createPFrame` instead of `createPFrameForGraphs`" and explains why `createPFrameForGraphs` was avoided (it would pull in `exports.properties` columns causing duplicates), but the code immediately below now calls `createPFrameForGraphs`. Any reader maintaining this file will be confused about which approach is intentional. The old justification no longer applies — the `isOutput !== "true"` guard and the `upstreamMeta` trace-filter together prevent the double-column issue — so the comment should be replaced with one that describes the new design.

### Issue 2 of 2
ui/src/pages/HistogramPage.vue:64-70
**`pl7.app/trace` checks are now dead code in both predicates**

`trace.inject` is no longer called when building `resultPframe` in `process.tpl.tengo`, so `propertiesPf` columns carry no `pl7.app/trace` annotation. Both predicates use `!spec.annotations?.["pl7.app/trace"]?.includes("sequence-properties")`, which evaluates to `!undefined` → `true` for every column and therefore has no filtering effect.

For `dataColumnPredicate` this is harmless, because `spec.annotations?.["pl7.app/isOutput"] === "true"` is the real gate. For `metaColumnPredicate` the dead check is the *only* guard, so the predicate unconditionally returns `true`, allowing sequence-properties numeric output columns (Net Charge, GRAVY, etc.) to appear in the meta/grouping picker alongside upstream sample-level metadata. If the intent was to keep outputs out of the meta picker, the predicate should test `isOutput` instead of (or in addition to) `trace`. The same issue exists in `ScatterPage.vue`.

_{Reviews (1): Last reviewed commit: "Changeset" | Re-trigger Greptile}

[gemini-code-assist]

Code Review

This pull request refactors the workflow's column specification logic into a dedicated library and updates the UI and model to enable linker usage for plot options. The review feedback highlights several critical issues: the removal of 'trace.inject' in the workflow causes a loss of provenance metadata, and the updated UI predicates for data columns are considered overly restrictive. Additionally, the reviewer noted that existing documentation in the model has become stale due to the implementation of 'createPFrameForGraphs' and needs to be updated to reflect the new approach.