Skip to content

Feat: new column access mechanism#1620

Open
AStaroverov wants to merge 1 commit into
mainfrom
feat/new-column-access-mechanism
Open

Feat: new column access mechanism#1620
AStaroverov wants to merge 1 commit into
mainfrom
feat/new-column-access-mechanism

Conversation

@AStaroverov

@AStaroverov AStaroverov commented May 7, 2026

Copy link
Copy Markdown
Collaborator

Greptile Summary

This PR introduces a new column-access mechanism for the Platforma SDK, replacing the previous ad-hoc column discovery with a structured ColumnsCollection abstraction backed by a host-side ColumnsCollectionDriverImpl. The new system supports anchored discovery (axis-aware linker traversal), selector filtering, spec override propagation, and a deduplication layer that collapses duplicate physical columns across sources.

  • New ColumnsCollectionDriver + ColumnsCollectionDriverImpl: opaque handle-based, refcounted collection objects; sources can be accessor trees, result-pool fan-outs, existing collections, or pre-resolved id lists. discover() and filter() produce derivative collections via the spec driver.
  • SpecOverrides / ColumnOverridedId layer: spec delta derivation, positional axis-patch merging, and collapseSpecOverrideNode that pushes overrides down to column leaves before handing the query to pframe-engine.
  • AccessorEntriesProvider / ResultPoolEntriesProvider: generic host-side and sandbox-side DFS-based column indexers with isFinal() derived from accessor lock state.

Confidence Score: 3/5

The core abstraction is well-designed, but two defects in the new code can cause incorrect behavior during normal progressive data loading.

Two independent defects both affect the non-final (loading) phase. First, resolveAnchors unconditionally throws when no anchor column has arrived in the collection yet; this turns a transient incomplete state into a hard computation error. Second, the module-level _accessorProviderCache in the SDK is keyed by the stable "main"/"staging" handle string, so it returns stale column entries on every render after the first and stops the computable framework from re-registering listInputFields as a reactive dependency — newly added columns are silently missed until something else triggers a re-run.

lib/model/columns-collection-driver/src/driver.ts (anchor resolution throwing) and sdk/model/src/columns/column_providers/providers.ts (stale LRU cache key)

Important Files Changed

Filename Overview
lib/model/columns-collection-driver/src/driver.ts New host-side ColumnsCollectionDriver implementation. Anchor resolution unconditionally throws when no anchors match, which will crash computables in transient non-final states.
sdk/model/src/columns/column_providers/providers.ts New sandbox-side ColumnsProvider. Module-level LRU cache keyed by the stable "main"/"staging" handle causes stale column entries and broken reactive dependency tracking across render cycles.
lib/model/common/src/drivers/pframe/spec/overrided.ts New SpecOverrides / ColumnOverridedKey system with spec delta derivation, merging, and application. Logic is well-structured; documented limitation that overrides cannot delete keys is intentional.
lib/node/pl-middle-layer/src/js_render/spec_override_collapse.ts New specOverride query-node collapser. Handles column, linkerJoin, and sliceAxes with correct positional-index translation. Non-handled node types throw with a clear message.
lib/model/common/src/drivers/columns/columns_collection_driver.ts Defines ColumnsCollectionDriver and ColumnsCollectionDriverModel interfaces. Clear separation of host and sandbox views; well-typed.
lib/model/common/src/columns/accessor_traversal.ts New DFS-based column enumeration utilities (findDescendantsByType, indexAccessorRoot, indexPoolBlock). Correct traversal through StdMap/StdMapSlash to PFrame leaves.
lib/model/common/src/drivers/pframe/spec/ids.ts Extended ColumnUniversalId / ColumnUniversalKey type hierarchy. reconstructSpecFromId correctly walks the id chain applying FilteredKey slicing and OverridedKey patch application.
lib/node/pl-middle-layer/src/js_render/service_injectors.ts New ColumnsCollection service injector. Correctly wires ColumnsCollectionDriverHost bindings (resolveAccessor, getUpstreamBlockCtxes, getSpecDriver, resolveSpec) to the computable context.

Sequence Diagram

sequenceDiagram
    participant Sandbox as Sandbox (SDK model)
    participant Bridge as VM Bridge (service_injectors)
    participant Driver as ColumnsCollectionDriverImpl
    participant Host as ColumnsCollectionDriverHost
    participant SpecDrv as PFrameSpecDriver

    Sandbox->>Bridge: columnsCollection.create(sources)
    Bridge->>Driver: create(sources, host)
    Driver->>Host: resolveAccessor / getUpstreamBlockCtxes
    Driver-->>Bridge: "PoolEntry<CollectionHandle>"
    Bridge-->>Sandbox: CollectionHandle (pinned to render ctx)

    Sandbox->>Bridge: "columnsCollection.discover(handle, {anchors, include})"
    Bridge->>Driver: discover(handle, options, host)
    Driver->>Host: resolveSpec(leafId) per id
    Driver->>SpecDrv: createSpecFrame(specMap)
    Driver->>Driver: resolveAnchors(anchors, specMap, specDrv)
    Driver->>SpecDrv: discoverColumns(frameKey, request)
    SpecDrv-->>Driver: DiscoverColumnsResponse
    Driver->>Driver: mapHitsWithDiscovery → ColumnUniversalId[]
    Driver-->>Bridge: "PoolEntry<CollectionHandle>"
    Bridge-->>Sandbox: new CollectionHandle

    Sandbox->>Bridge: columnsCollection.getColumns(handle)
    Bridge->>Driver: getColumns(handle, host)
    Driver->>Driver: dedupColumns(all contributions)
    Driver-->>Bridge: ColumnUniversalId[]
    Bridge-->>Sandbox: ColumnUniversalId[]
Loading
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
lib/model/columns-collection-driver/src/driver.ts:438-440
**`resolveAnchors` throws in transient states, crashing the computable**

When `discover()` is called with anchors but none of the specified anchor columns have arrived in the collection yet (e.g. an upstream block hasn't published its PFrame, but other upstream blocks have, so `ids.length > 0` passes the early-exit guard), `resolveAnchors` throws `"At least one anchor must be resolved to a valid column"`. This propagates as an uncaught exception through the QuickJS bridge, putting the computable into an error state rather than an incomplete/pending state. The correct behaviour in a non-final, non-empty collection is to return an empty minted collection (as is already done on line 258 for the zero-ids case).

The fix would be to return early in `runDiscovery` when `resolveAnchors` produces zero matches:

### Issue 2 of 2
sdk/model/src/columns/column_providers/providers.ts:57-67
**Stable "main"/"staging" cache key causes stale column entries across render cycles**

`_accessorProviderCache` is keyed by `root.handle`. For the well-known accessors the host always returns the string `"main"` or `"staging"` as the handle (from `getAccessorHandleByName`). This key is stable across render cycles even though the underlying accessor state can change (e.g. new columns added to outputs while the block is still running). On the second render the cache hits, `AccessorEntriesProvider`'s constructor is not re-run, so `indexAccessorRoot` / `listInputFields` are not called again. Two consequences: (1) newly-added columns are absent from `entries`, and (2) the computable framework does not re-register `listInputFields("main")` as a dependency, so it may not re-trigger when further columns arrive. A content-based key (e.g. incorporating the accessor's locked state or a snapshot hash) or not caching across render cycles would fix this.

Reviews (4): Last reviewed commit: "feat: new column access mechanism" | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

@changeset-bot

changeset-bot Bot commented May 7, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e765c4f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 32 packages
Name Type
@milaboratories/columns-collection-driver Minor
@milaboratories/milaboratories.ui-examples.ui Minor
@milaboratories/pl-middle-layer Minor
@milaboratories/pf-driver Minor
@milaboratories/pl-model-common Minor
@milaboratories/pl-tree Minor
@platforma-sdk/ui-vue Minor
@platforma-sdk/model Minor
@milaboratories/milaboratories.ui-examples Patch
@milaboratories/pl-mcp-server Major
@platforma-sdk/pl-cli Patch
@platforma-sdk/test Minor
@milaboratories/pl-model-middle-layer Patch
@milaboratories/pf-spec-driver Patch
@milaboratories/pl-client Patch
@milaboratories/pl-drivers Patch
@milaboratories/pl-deployments Patch
@platforma-open/milaboratories.software-ptabler.schema Patch
@platforma-sdk/block-tools Patch
@milaboratories/milaboratories.monetization-test.ui Patch
@milaboratories/milaboratories.pool-explorer.ui Patch
@milaboratories/uikit Patch
@milaboratories/milaboratories.monetization-test.model Patch
@milaboratories/milaboratories.ui-examples.model Patch
@milaboratories/milaboratories.pool-explorer.model Patch
@milaboratories/milaboratories.pool-explorer Patch
@milaboratories/pl-model-backend Patch
@milaboratories/pl-errors Patch
@platforma-sdk/bootstrap Patch
@milaboratories/ptabler-expression-js Patch
@milaboratories/milaboratories.monetization-test Patch
@platforma-sdk/tengo-builder Patch

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 gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors column handling and result pool logic to support lazy data status and efficient object discovery. It introduces PObjectFieldStatus to decouple data availability from accessor materialization and implements a raw result pool view for upstream blocks. The SDK has been updated to use the standard PColumn interface instead of ColumnSnapshot. Review feedback identifies critical bugs in AccessorColumnProvider regarding nested structure resolution and in isPColumnReady where readiness checks are skipped for "presented" status. Additionally, there is a fragile import from a dist directory and a logic error in mapping data status for resolving TreeNodeAccessor objects.

Comment thread sdk/model/src/columns/column_providers/providers.ts Outdated
Comment thread sdk/model/src/render/util/pcolumn_data.ts Outdated
Comment thread lib/node/pl-tree/src/accessors.ts Outdated
Comment thread sdk/model/src/components/PlDataTable/createPlDataTable/utils.ts Outdated
Comment thread sdk/model/src/columns/column_providers/providers.ts Outdated
Comment thread lib/node/pl-tree/src/accessors.ts Outdated
@AStaroverov AStaroverov force-pushed the feat/new-column-access-mechanism branch from 0601340 to 44f864b Compare May 7, 2026 17:54
@AStaroverov

Copy link
Copy Markdown
Collaborator Author

@greptileai

@codecov

codecov Bot commented May 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 38.17992% with 591 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.60%. Comparing base (a5bc059) to head (e765c4f).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
.../model/common/src/drivers/pframe/spec/overrided.ts 0.00% 106 Missing ⚠️
...ddle-layer/src/js_render/spec_override_collapse.ts 0.00% 44 Missing ⚠️
sdk/model/src/columns/column_lazy.ts 56.32% 34 Missing and 4 partials ⚠️
sdk/model/src/columns/derive_axis_values_labels.ts 5.88% 32 Missing ⚠️
sdk/model/src/columns/utils.ts 45.76% 30 Missing and 2 partials ⚠️
...dk/model/src/columns/column_providers/providers.ts 26.82% 28 Missing and 2 partials ⚠️
lib/model/common/src/drivers/pframe/spec/ids.ts 0.00% 27 Missing ⚠️
...pl-middle-layer/src/js_render/service_injectors.ts 0.00% 27 Missing ⚠️
sdk/model/src/columns/columns_collection.ts 40.47% 18 Missing and 7 partials ⚠️
lib/model/common/src/pool/spec.ts 0.00% 23 Missing ⚠️
... and 27 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1620      +/-   ##
==========================================
- Coverage   53.07%   52.60%   -0.48%     
==========================================
  Files         270      284      +14     
  Lines       15719    16282     +563     
  Branches     3411     3606     +195     
==========================================
+ Hits         8343     8565     +222     
- Misses       6262     6579     +317     
- Partials     1114     1138      +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread lib/node/pf-driver/src/driver_double.test.ts Outdated
Comment thread lib/node/pf-driver/src/driver_double.test.ts Outdated
Comment thread lib/node/pl-middle-layer/src/js_render/computable_context.ts
Comment thread lib/node/pl-middle-layer/src/js_render/computable_context.ts Outdated
Comment thread lib/node/pl-middle-layer/src/js_render/computable_context.ts Outdated
Comment thread lib/node/pl-middle-layer/src/js_render/computable_context.ts
Comment thread lib/node/pl-middle-layer/src/js_render/computable_context.ts
Comment thread lib/node/pl-tree/src/accessors.ts Outdated
Comment thread lib/node/pl-tree/src/accessors.ts Outdated
Comment thread sdk/model/src/columns/column_providers/index.ts Outdated
Comment thread sdk/model/src/columns/expand_by_partition.test.ts
Comment thread sdk/model/src/render/accessor.ts Outdated
Comment thread sdk/model/src/render/api.ts Outdated
@AStaroverov AStaroverov force-pushed the feat/new-column-access-mechanism branch 3 times, most recently from 40ebdb7 to 61afec9 Compare May 20, 2026 15:08
@AStaroverov

Copy link
Copy Markdown
Collaborator Author

@greptileai

Comment thread lib/model/columns-collection-driver/src/driver.ts
Comment thread lib/model/common/src/columns/accessor_traversal.ts Outdated
Comment thread lib/model/columns-collection-driver/src/driver.ts Outdated
Comment thread lib/model/columns-collection-driver/src/driver.ts Outdated
Comment thread lib/model/columns-collection-driver/src/driver.ts Outdated
@AStaroverov AStaroverov force-pushed the feat/new-column-access-mechanism branch from b3f266c to c7309fc Compare May 22, 2026 15:09
@AStaroverov

Copy link
Copy Markdown
Collaborator Author

@greptileai

@AStaroverov AStaroverov force-pushed the feat/new-column-access-mechanism branch from d698e40 to e222669 Compare May 28, 2026 15:18
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@AStaroverov AStaroverov force-pushed the feat/new-column-access-mechanism branch from e222669 to e765c4f Compare May 28, 2026 15:23
@AStaroverov

Copy link
Copy Markdown
Collaborator Author

@greptileai

Comment on lines +438 to +440
if (resolvedIds.size === 0) {
throwError("At least one anchor must be resolved to a valid column");
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 resolveAnchors throws in transient states, crashing the computable

When discover() is called with anchors but none of the specified anchor columns have arrived in the collection yet (e.g. an upstream block hasn't published its PFrame, but other upstream blocks have, so ids.length > 0 passes the early-exit guard), resolveAnchors throws "At least one anchor must be resolved to a valid column". This propagates as an uncaught exception through the QuickJS bridge, putting the computable into an error state rather than an incomplete/pending state. The correct behaviour in a non-final, non-empty collection is to return an empty minted collection (as is already done on line 258 for the zero-ids case).

The fix would be to return early in runDiscovery when resolveAnchors produces zero matches:

Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/model/columns-collection-driver/src/driver.ts
Line: 438-440

Comment:
**`resolveAnchors` throws in transient states, crashing the computable**

When `discover()` is called with anchors but none of the specified anchor columns have arrived in the collection yet (e.g. an upstream block hasn't published its PFrame, but other upstream blocks have, so `ids.length > 0` passes the early-exit guard), `resolveAnchors` throws `"At least one anchor must be resolved to a valid column"`. This propagates as an uncaught exception through the QuickJS bridge, putting the computable into an error state rather than an incomplete/pending state. The correct behaviour in a non-final, non-empty collection is to return an empty minted collection (as is already done on line 258 for the zero-ids case).

The fix would be to return early in `runDiscovery` when `resolveAnchors` produces zero matches:

How can I resolve this? If you propose a fix, please make it concise.

@mzueva mzueva self-requested a review June 15, 2026 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants