refactor(workflow-executor): rebuild field displayNames dynamically#1618
Open
PMerlet wants to merge 9 commits into
Open
Conversation
Persist only technical names (fieldName / action name) in step execution data and re-derive displayName from the collection schema at read time, so labels edited in the rendering never go stale on replay or resume. - step-execution-data: strip displayName from persisted refs; add a Displayed*/Hydrated view union for read-time consumption - executors (update/trigger/load-related/read): stop persisting displayName - add hydrateStepExecutionData() + makeSchemaGetter(), used by runner.getRunStepExecutions and the AI step summaries - drop displayName from the load-related-record PATCH validator No DB migration: execution data is a single JSON column and old rows with a leftover displayName still read fine (the technical name is the source of truth). fixes PRD-426 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
5 new issues
|
… into feature/prd-426-always-rebuild-field-displaynames-dynamically # Conflicts: # packages/workflow-executor/src/executors/load-related-record-step-executor.ts # packages/workflow-executor/src/http/pending-data-validators.ts
…ield and fallthrough paths Adds cases for the awaiting-input / executing / automatic phases (where executionParams or pendingData is absent), the relation-result undefined branch, the action-vs-field lookup, and the unrecognized-type fallthrough. Brings hydrate-step-execution-data.ts to 100% statement/branch/function/line coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eads QA hardening of the read-time hydration path: - De-duplicate concurrent schema fetches: makeSchemaGetter now shares an in-flight promise per collection, so hydrating N steps of one collection (Promise.all in getRunStepExecutions / buildPreviousStepsMessages) triggers a single orchestrator call instead of N. - Tolerate malformed/legacy rows: a throw while hydrating one execution is caught and the raw execution returned, so one bad row no longer 500s the whole GET /runs/:runId response. - Observability: schema-fetch failures (warn) and per-row hydration failures (error) are now logged with collection/type/stepIndex context instead of failing silently. Tests cover the stampede de-dup, failed-fetch retry/no-cache, malformed-row resilience, and both log paths. hydrate-step-execution-data.ts stays at 100% coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…oss-rendering label leaks displayName, and the visible field/action set, are rendering-specific (forestadmin-server resolves them from the run's rendering layout). The process-wide SchemaCache was keyed by collectionName only, so within one environment a run on rendering A could serve its labels / visible fields to a run on rendering B for the cache TTL — wrong labels and, when a rendering hides fields, a wrong field set fed to the AI. - SchemaCache is now keyed by (renderingId, collectionName) via nested maps; the bare iterator is replaced by entriesForRendering(renderingId). - makeSchemaGetter takes renderingId (cache scope) alongside runId (server fetch, which resolves the run's rendering). RecordStepExecutor.getCollectionSchema delegates to it for consistent keying + dedup; buildPreviousStepsMessages and getRunStepExecutions pass the rendering. - getRunStepExecutions(runId, renderingId): the HTTP read path passes the requesting user's renderingId. AgentClientAgentPort scopes resolveSchema/buildActionEndpoints by user.renderingId. Technical identifiers (fieldName, action name, primary keys, endpoints) are environment-level and were never corrupted; this fix isolates the rendering-scoped labels and field membership. Tests: SchemaCache rendering isolation + entriesForRendering, makeSchemaGetter per-rendering isolation, and an end-to-end runner test proving two renderings sharing one cache don't contaminate. schema-cache.ts and hydrate-step-execution-data.ts at 100% coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…f on reads Record that label edits can take up to the 10-minute TTL to surface on the hydration read path as an accepted trade-off, so it reads as intentional rather than a defect. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
…read route Addresses Macroscope review: handleGetRun cast the JWT payload to StepUser and passed renderingId downstream without runtime validation. A token missing renderingId (or with a non-numeric value) would send undefined/NaN into the schema cache. Mirror the bearerUserId guard used in handleTrigger: coerce to number, and return 400 when it is not finite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…r-getter The in-flight de-dup previously lived on each getter (one per getRunStepExecutions call / per executor step), so it only collapsed concurrent misses within a single read. Two independent concurrent readers of the same cold (rendering, collection) — two reads of one run, or two runs of one rendering — still stampeded the orchestrator until the first fetch resolved. Move the read-through + de-dup into SchemaCache.getOrFetch, keyed by (renderingId, collectionName) on the one shared cache. All callers now share a single in-flight promise per (rendering, collection); makeSchemaGetter is a thin wrapper. A rejected fetch is not cached and clears the in-flight slot so the next caller retries. Tests: getOrFetch (hit/miss/concurrent-share/per-rendering/failed-retry) and a cross-getter global de-dup test. schema-cache.ts and hydrate-step-execution-data.ts at 100% coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ration code Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes PRD-426
What & why
The executor persisted field/action/relation
displayNamein step execution data. Labels come from the per-rendering layout and change, so replayed/resumed runs showed stale labels. Now we persist only the technical name (fieldName/ actionname) and rebuilddisplayNamefrom the current schema on read (GET /runs/:runId+ AI step summaries).No DB migration — execution data is a single JSON column; old rows with a leftover
displayNameread fine (the technicalnameis the source of truth).Core change
step-execution-data.ts: refs are technical-name-only; added aDisplayed*/HydratedStepExecutionDataview for reads.displayName; newhydrate-step-execution-data.tsre-injects it from the schema.runner.getRunStepExecutionsandbuildPreviousStepsMessageshydrate on read; load-related PATCH validator dropsdisplayName.QA hardening
SchemaCache.getOrFetch— concurrent callers share one fetch (no stampede).(renderingId, collectionName);displayNameand visible fields are rendering-specific, so the oldcollectionName-only key leaked labels across renderings in one environment.renderingIdvalidation on the read route (400 if missing/invalid) — Macroscope fix.Tests
877 passing / 40 suites, lint clean;
hydrate-step-execution-data.ts+schema-cache.tsat 100%. Covers changed-schema rebuild, malformed rows, fetch de-dup, and cross-rendering isolation.Known limitations (intentional)
renderingId— correct for viewing your own run; a cross-rendering view is bounded.getRelatedDataPK-fallback-to-['id']on cache miss — pre-existing, WIP related-data code, left as-is.Targets
feat/prd-214-server-step-mapper(package not onmainyet).Note
Rebuild field displayNames dynamically from schema at read time instead of persisting them
displayNamefrom all persisted step execution data (FieldRef,ActionRef,RelationRef) across executors forread-record,update-record,trigger-action, andload-related-recordsteps.hydrateStepExecutionDatafunction inhydrate-step-execution-data.tsthat re-derives display names from the current schema at read time, with graceful fallback to technical names on fetch or hydration failures.HydratedStepExecutionDataandDisplayed*type variants instep-execution-data.tsto represent the enriched view returned to callers.SchemaCachebyrenderingIdwith concurrent fetch deduplication viagetOrFetch, and updatesRunner.getRunStepExecutionsto requirerenderingIdas a second argument.GET /runs/:runIdnow returns 400 if the bearer token lacks a validrenderingId; persisted step execution records no longer containdisplayNamefields.Macroscope summarized be74367.