Discussion: Unify data access patterns for vary dimensions

[stack72]

Context

While designing the vary dimensions feature (#384), we identified a tension between the two CEL data access patterns that makes vary harder to implement cleanly.

The Two Access Patterns Today

Pattern	Sees current-run data?	Sees prior-run data?	Syntax
model["name"].resource.spec.data	Yes — updated in-memory after each step	Yes	Dictionary traversal
data.latest("name", "data")	No — snapshot from workflow start	Yes	Function call

Both exist intentionally:

• model.* is mutable/live — enables step-to-step chaining within a workflow run
• data.latest() is an immutable snapshot — provides consistency and isolation for cross-run access

The Problem with Vary

The vary feature (#384) needs a way to dynamically resolve the composite dataName from vary dimensions. A function call can do this naturally:

${{ data.latest("deploy-app", "result", [inputs.environment]).attributes.exitCode }}

But model.* is dictionary traversal — there's no function to extend. Within a workflow, you'd be stuck with manual string concatenation:

${{ model["deploy-app"].resource["result"]["result-" + inputs.environment].attributes.exitCode }}

This is exactly the manual name construction that vary is designed to eliminate.

The fundamental tension: the access pattern that benefits from vary (data.latest()) doesn't see current-run data, and the one that sees current-run data (model.*) can't benefit from vary.

Proposal: Make data.latest() see current-run data

If data.latest() were updated after each workflow step (same as the model context is today), it could serve as the single accessor that supports both vary and current-run data:

# Within a workflow — sees current-run data, with vary support
${{ data.latest("deploy-app", "result", [inputs.environment]).attributes.exitCode }}

# Cross-workflow — sees historical data, with vary support
${{ data.latest("deploy-app", "result", [inputs.environment]).attributes.exitCode }}

# Same syntax works in both cases

The model.* dot-notation would still exist for simple cases where vary isn't needed:

# Simple access without vary — still works
${{ model["deploy-app"].resource.result.result.attributes.exitCode }}

What this means for implementation

• After each workflow step completes, update the data cache (same as the model context update that already happens in execution_service.ts)
• data.latest() shifts from a pure snapshot to a live view within workflow execution
• Outside of workflows (e.g., swamp model evaluate), behavior stays the same — reads from disk

Trade-offs to discuss

For:

• Single accessor that supports both vary and current-run data
• No manual string concatenation for vary dimensions
• data.latest() becomes more useful within workflows
• Consistent behavior — data.latest() always returns the most recent data, whether from this run or a prior run

Against:

• Changes the snapshot isolation guarantee of data.latest() within workflows
• Could cause surprises if users expect data.latest() to be stable throughout a workflow run
• Adds complexity to the data cache (needs to support mid-run updates)
• Two concurrent workflow runs could see each other's intermediate state if they share the same data cache (though this is already true for model.*)

Questions for Discussion

1. Is the snapshot isolation of data.latest() something users depend on, or is it an implementation detail?
2. Should the live behavior be opt-in (e.g., a separate function like data.current()) rather than changing data.latest()?
3. Are there other approaches we haven't considered?

Related: #384, #385, #386, #388, #389

[stack72]

Implementation Plan: Unify Data Access — Make data.latest() the Canonical Accessor

Analysis

After investigating the codebase, we've identified why both patterns exist and what needs to change:

The DataCache is an implementation accident, not a design choice. It exists because CEL evaluation (cel-js) is synchronous and all repository methods are async. The cache pre-loads everything into memory so CEL functions can do synchronous lookups. But Deno provides sync filesystem APIs (Deno.readTextFileSync, Deno.readLinkSync, Deno.readDirSync), and the codebase already uses Deno.statSync and Deno.readTextFileSync in the same file for file metadata and file.contents(). So the cache is unnecessary complexity.

Recommendation: Remove the DataCache entirely. Replace with direct sync disk reads from CEL functions. This makes data.latest() always return fresh data — including data written by previous steps in the current workflow run.

---

Implementation Steps

Step 1: Add sync read methods to FileSystemUnifiedDataRepository

File: src/infrastructure/persistence/unified_data_repository.ts

Add to the UnifiedDataRepository interface and implement:

• getLatestVersionSync(type, modelId, dataName): number | null — reads latest symlink via Deno.readLinkSync, falls back to Deno.readDirSync scan
• findByNameSync(type, modelId, dataName, version?): Data | null — resolves version via getLatestVersionSync if needed, reads metadata.yaml via Deno.readTextFileSync, parses YAML
• listVersionsSync(type, modelId, dataName): number[] — scans version dirs via Deno.readDirSync
• getContentSync(type, modelId, dataName, version?): Uint8Array | null — reads raw file via Deno.readFileSync
• findAllForModelSync(type, modelId): Data[] — scans data-name dirs via Deno.readDirSync, calls findByNameSync for each

Path computation is already sync: reuse existing getPath(), getContentPath().

Step 2: Remove DataCache, add ModelCoordinatesMap, rewrite data.* functions

File: src/domain/expressions/model_resolver.ts

• Delete the DataCache class entirely
• Define a ModelCoordinatesMap (Map<string, {modelType, modelId}[]>) that maps definition names to their disk coordinates
• In buildContext(), build the coordinates map from definitions + orphan recovery via findAllGlobal()
• Still populate model.*.resource and model.*.file eagerly for backward compatibility
• Replace data.* namespace functions with sync disk reads using the coordinates map

The data.* functions become:

• data.latest(modelName, dataName) — look up coordinates, call sync repo methods, parse JSON content for attributes
• data.version(modelName, dataName, version) — same but with explicit version
• data.listVersions(modelName, dataName) — call listVersionsSync()
• data.findByTag(tagKey, tagValue) — scan all coords, read metadata sync, filter by tag
• data.findBySpec(modelName, specName) — scan coords for model, filter by specName tag

Step 3: Add deprecation warning for model.*.resource / model.*.file

File: src/infrastructure/cel/cel_evaluator.ts

• Add deprecation detection in CelEvaluator.evaluate() using regex
• Deduplicate warnings with a Set<string> so each pattern warns once per evaluator instance
• Log via getLogger(["expressions"]).warn (matches existing vault deprecation pattern)

NOT deprecated: model.*.input, model.*.definition, model.*.execution — these are model metadata, not versioned data

Step 4: Update tests

• Remove DataCache unit tests, add tests for sync disk reads
• Add integration test: write data after buildContext(), verify data.latest() returns fresh data
• Add test for deprecation warning emission

Step 5: Update documentation

• Skill docs: mark data.latest() as preferred, model.*.resource as deprecated
• Design docs: note data.latest() is canonical

---

Why This is the Right Move

Simplicity of reasoning. Today, users and contributors have to think about "which accessor sees what data when." After this change, data.latest() always means "read the latest from disk" — no hidden staleness, no snapshot timing to reason about. The behavior is obvious.

Alpha is the right time. The user base is small, and this kind of change only gets harder to make later. We keep backward compatibility with deprecation warnings so nothing breaks immediately.

The alternative — keeping the cache but updating it after each step — is worse. It would leave the unnecessary abstraction in place, add update logic, and make the code harder to reason about ("is the cache up to date? when was it last refreshed?"). Removing it entirely is cleaner.

---

Known Performance Trade-off: findByTag() / findBySpec()

findByTag() and findBySpec() will scan all metadata files on disk synchronously on every call. This is O(total_data_items) per invocation. On a repo with hundreds of data versions, this could feel slow.

Why this is acceptable for now:

• We are in alpha — correctness and simplicity are more important than performance
• These functions are rarely used in practice compared to data.latest() and data.version()
• The scan is synchronous but the data is local filesystem (not network I/O)
• If this becomes a problem, the fix is well-understood: build a lightweight tag index during the findAllGlobal() pass in buildContext() and use it for sync lookups. But adding that now would re-introduce cache staleness concerns, so we'll only do it when we have evidence it's needed.