fix(plan-level): AB#3156 — research path StrictUndefined + PowerShell bareword - #354
Merged
Merged
Conversation
… bareword Two latent bugs in the research path of plan-level.yaml (introduced by PR #351, never caught in CI because harness Integration scenarios are excluded from the fast loop). Both fired in the AB#3127 dogfood at 14:42 PT today, killing the second relaunch attempt. ## Bug 1: research_loop_counter $hasTopics = true (bareword) The PowerShell script rendered the Jinja boolean unquoted. PowerShell parsed the bareword `true` as a cmdlet name, errored, and left $hasTopics = $null. JSON-serialized as null, which jinja treats as null != false, routing to research_dispatch instead of open_questions_policy even when the architect emitted empty topics. Fix: string-compare the rendered token. ## Bug 2: research_dispatch escalation_cap StrictUndefined The architect output schema declares research_needs as a typed object but doesn't always populate every key. escalation_cap was a newer optional addition (AB#3134) the architect never emits. Under StrictUndefined (conductor default), dict.missing_attr raises UndefinedError BEFORE the default() filter sees the value, so the filter never fires. Fix: use Python dict.get() for all optional research_needs keys — escalation_cap, budget_hint, and the nested context.plan_excerpt / archive_scope.paths / archive_scope.tags. Bug 1 chain-fires Bug 2: even when topics is empty (no research path), the broken $hasTopics routing forwards to research_dispatch, which then hits escalation_cap. Both must be fixed together. ## Verification - conductor validate plan-level.yaml — clean - harness scenario architect_research_loop — PASSES end-to-end (was failing on main with the same errors). Full loop now executes through to workflow_completed. ## Harness scenario adjustment Updated the architect_research_loop scenario fixture to emit the full research_needs shape (topics + context + budget_hint + archive_scope) on the second architect call, matching what the real architect emits. The minimal {topics: []} shape hit conductor's recursive required-by-default validator (orthogonal to the bugs above). The deeper output-schema-strictness issue is a conductor design point worth revisiting in a follow-up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced May 14, 2026
PolyphonyRequiem
pushed a commit
that referenced
this pull request
May 14, 2026
…kind schema Refactor the architect's research payload from a nested-optional `research_needs` object to a flat discriminated-union shape with a required `research_request_kind` discriminant and always-present sibling `research_*` fields. Eliminates the AB#3156 trap at the source: routing is now a string compare on a guaranteed-present field (`architect.output.research_request_kind == 'request'`) and the input_mapping reads flat top-level fields directly — no `.get(key, default)` chains, no StrictUndefined hazard. Why --- Conductor enforces "all declared schema properties are required" at runtime, but the previous schema declared `research_needs` as an *optional* nested object whose own properties (`topics`, `context`, `budget_hint`, `archive_scope`) were also optional. Every consumer in plan-level.yaml had to defensively chain `.get(key, default)` and a single missed `.attr | default()` (instead of `.get()`) crashed the run via StrictUndefined (see AB#3156 / PR #354). The fix is a discriminated-union lean: replace the nested-optional shape with a flat top-level discriminant + sibling fields the architect always emits. Schema before / after --------------------- Before (architect output, plan-level.yaml): research_needs: # optional topics: [string] # optional under it context: { ... } # optional, nested deeper budget_hint: string # optional archive_scope: { ... } # optional After: research_request_kind: "none" | "request" # required, ALWAYS present research_topics: [string] # always emitted; [] when "none" research_context: object # always emitted; {} when "none" research_budget_hint: string # always emitted; "" when "none" research_archive_scope: object # always emitted; {} when "none" research_escalation_cap: number # always emitted; default 1 Routing before / after ---------------------- Before (research_loop_counter routes): when: ""{{ research_loop_counter.output.has_topics == false }}"" …where `has_topics` was computed by a PowerShell bareword guard: \ = '{{ (architect.output.research_needs.topics | default([]) | length > 0) | string | lower }}' -eq 'true' After: when: ""{{ architect.output.research_request_kind == 'none' }}"" The PowerShell `has_topics` guard is gone — routing reads the always-present discriminant directly. The `research_dispatch` input_mapping now reads `architect.output.research_topics` / `research_budget_hint` / `research_escalation_cap` directly; `research_context` and `research_archive_scope` keep `.get()` since their nested keys are described as opaque `type: object` in the output schema (so consumers tolerate `{}` without recursive nested validation rejecting it). Files ----- - `.conductor/registry/prompts/architect-plan-level.md` — schema example, `Identify research needs` table, and the `research_needs` field section all updated to the flat shape. Documents both `kind= one` and `kind= equest` examples explicitly so the LLM has a concrete pattern. - `.conductor/registry/workflows/plan-level.yaml` — output schema flipped to the flat shape (`research_context` / `research_archive_scope` are opaque `type: object` so empty `{}` validates under conductor's recursive-required policy); `research_loop_counter` route condition simplified to a string compare; PowerShell `has_topics` guard removed; `research_dispatch` input_mapping reads flat fields. - `tests/harness/scenarios/architect_research_loop/scenario.yaml` — both architect fixture calls updated. 1st call: `research_request_kind: ""request""` triggers research_dispatch. 2nd call: `research_request_kind: ""none""` falls through to open_questions_policy (verified by the harness scenario passing end-to-end with all 5 expected agents executed in order). Verification ------------ - `conductor validate .conductor/registry/workflows/plan-level.yaml` → Validation Successful. - `Invoke-Pester tests/harness/run-scenarios.Tests.ps1` → all 5 scenarios green (architect_research_loop, cascade_remedy_no_stale, close_out_happy_path, research_happy_path). - `Invoke-Pester tests/lint-jinja-resolver.Tests.ps1` → 30/30 green. - `Invoke-Pester .conductor/registry/tests/lint-strict-undefined.Tests.ps1` → 11/11 green. The lint-plan-level.ps1 `missing-oq-answer-counter-node` failure is pre-existing on `main` (verified) and unrelated to this change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Merged
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.
Summary
Fixes AB#3156 — two latent bugs in
plan-level.yaml's research path that PR #351 introduced and never caught in CI (harness Integration scenarios are excluded from the fast loop). Both fired in the AB#3127 dogfood at 14:42 PT today, killing the second relaunch attempt after PR #353 fixed the architect-prompt regression.Bug 1:
research_loop_counter$hasTopics = truebarewordThe PowerShell script rendered the Jinja boolean unquoted, so PowerShell parsed
trueas a cmdlet name and left$hasTopics = $null. JSON-serialized asnull, which jinja treats asnull != false, routing toresearch_dispatchinstead ofopen_questions_policyeven when the architect emitted empty topics.Fix: string-compare the rendered token.
Bug 2:
research_dispatchescalation_capStrictUndefinedThe architect output schema declares
research_needsas a typed object but doesn't always populate every key.escalation_capwas a newer optional addition (AB#3134) the architect never emits.Under StrictUndefined (conductor's default),
dict.missing_attrraisesUndefinedErrorBEFORE thedefault()filter sees the value, so the filter never fires.Fix: use Python
dict.get()for all optionalresearch_needskeys —escalation_cap,budget_hint, and the nestedcontext.plan_excerpt/archive_scope.paths/archive_scope.tags.{{ research_needs.get('escalation_cap', 1) }} {{ research_needs.get('context', {}).get('plan_excerpt', '') }}Bug 1 chain-fires Bug 2 — even when topics is empty (no-research path), the broken
$hasTopicsrouting forwards toresearch_dispatch, which then hitsescalation_cap. Both must be fixed together.Harness scenario adjustment
Updated the
architect_research_loopscenario fixture to emit the fullresearch_needsshape (topics + context + budget_hint + archive_scope) on the second architect call, matching what the real architect emits. The minimal{topics: []}shape hit conductor's recursive required-by-default validator (orthogonal to the bugs above).Verification
conductor validate plan-level.yaml— cleanarchitect_research_loopharness scenario — passes end-to-end, full loop executes toworkflow_completed. Was failing onmainwith both bugs visible in output.Follow-up
The recursive required-by-default behavior of conductor's output validator is a deeper design point worth revisiting (every architect emit must perfectly match the schema's recursive shape, including nested objects). Out of scope here.