fix(workflow): load-related chooses the relation by its target collection#1630
Open
Scra3 wants to merge 5 commits into
Open
fix(workflow): load-related chooses the relation by its target collection#1630Scra3 wants to merge 5 commits into
Scra3 wants to merge 5 commits into
Conversation
…tion A load-related step picked the source record first, by "which available record's collection best matches the request", then the relation on it. With records accumulated from prior load-related steps, "load the dvd X" latched onto an already-loaded dvd (collection matches) instead of the store that has the dvds relation — so it followed dvd→store and re-loaded a store. Fuse the two choices into one: gather every (record, relation) pair across available records and pick by what the relation LEADS TO. "Load the dvd X" now follows store→dvds regardless of accumulated dvds. preRecordedArgs (selectedRecordStepIndex / relationDisplayName) still pin source/relation; single candidate skips the AI call. The shared select-record heuristic is untouched for read/update/trigger (acting on an existing record of that type). NOTE: load-related unit tests still mock the old select-relation flow — to be migrated in a follow-up commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 new issues
|
The orchestrator sends a `title` on every step (ServerWorkflowStepBase.title) but the mapper dropped it. Carry it into the domain StepDefinition and inject it into buildContextMessage, so every AI call sees the step's human intent — which matters when the prompt is weak or empty (e.g. a load-related step whose prompt arrives as "Load " while the title says "Load the store"). - add optional `title` to shared StepDefinition fields - map `task.title` / `condition.title` in step-definition-mapper - buildContextMessage appends `Step title: "<title>"` when present Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tion
Adapt the suite to the single `select-relation-to-follow` AI call (was
`select-record` + `select-relation`): mocks return `{ relation: <label> }`,
single-candidate schemas make no AI call, bindTools counts/names updated.
Adds a repro test: with account + a loaded store + a loaded dvd available,
"Load the dvd titanic" follows store→dvds (not a relation off the dvd).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Coverage Impact This PR will not change total coverage. Modified Files with Diff Coverage (3)
🛟 Help
|
Add coverage for the diff lines flagged by the coverage gate: the step-title branch in buildContextMessage, and resolveTarget's requireRecordAtStepIndex (pinned source via selectedRecordStepIndex, plus the no-match error path). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR review findings:
- a pinned relationDisplayName / selectedRecordStepIndex that matches nothing
now throws InvalidPreRecordedArgsError, not the misleading
NoRelationshipFieldsError ("no relations configured"); the latter is kept
only for the genuine zero-relations case.
- matchesRelation normalizes (case + separators) like findField, restoring
fuzzy parity for pre-recorded relation names.
- resolveTarget builds the RelationTarget straight from the chosen candidate's
field (targetFromCandidate) instead of re-resolving by displayName via
buildTarget — removes a redundant lookup and a same-displayName mis-resolve
hazard. RelationCandidate.field is narrowed to guarantee relatedCollectionName.
- tests: pinned-relation/source assertions made meaningful (executionParams +
source record among several), error userMessages asserted, and a test that
the step title reaches the select-relation-to-follow context.
Co-Authored-By: Claude Opus 4.8 (1M context) <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.
Context
Stacked on #1628 (base
fix/prd-442-activity-log-target). Two distinct load-related improvements.1. Choose the relation by its target collection
A load-related step picked the source record first (« which available record's collection best matches the request »), then the relation. With records accumulated from prior load-related steps, « Load the dvd X » latched onto an already-loaded dvd (collection matches) instead of the store that has the
dvdsrelation → it followeddvd → storeand re-loaded a store.Fix: fuse the two choices into one over
(record, relation)pairs, picked by what the relation leads to. New AI toolselect-relation-to-follow(options labeledSource → relation (→ target)) replacesselect-record+select-relationfor load-related.preRecordedArgs(selectedRecordStepIndex/relationDisplayName) still pin source/relation; a single candidate skips the AI call. The sharedselect-recordheuristic is untouched for read/update/trigger.2. Thread the step title to the AI
The orchestrator sends a
titleon every step (ServerWorkflowStepBase.title) but the mapper dropped it. It's now mapped intoStepDefinitionand injected intobuildContextMessage, so every AI call sees the step's intent — important when the prompt is weak/empty (e.g. a load-related prompt arriving as « Load » while the title says « Load the store »).Verified live (
_example)account→ load store → read store name → load dvd "Celine dion" → load dvd "titanic": the last step followsstore#6 → dvds(GET /store/6/relationships/dvds, candidates incl. Titanic), no moredvd → storetrap.Tests
select-relation-to-followcall + a repro test (account + loaded store + loaded dvd → "Load the dvd titanic" follows store→dvds, assertsgetRelatedData(collection: 'store', relation: 'dvds')).titleis carried through.🤖 Generated with Claude Code
Note
Fix load-related step to select relations by target collection
LoadRelatedRecordStepExecutorrelation selection to build candidates from all available records and choose by target collection name, rather than by source record resemblance.resolveTarget()to consolidate record and relation resolution; skips AI when only one eligible candidate exists.SELECT_RELATION_SYSTEM_PROMPTto instruct the AI to choose relations by their target collection.titlefield to step definitions and includes it in the system context message built byBaseStepExecutor.selectedRecordStepIndexnow throwInvalidPreRecordedArgsErrorinstead of falling through silently; the AI tool for relation selection is renamed toselect-relation-to-followand requires choosing from explicit labeled options.Macroscope summarized c264c51.