Skip to content

perf(od-next): streamline tool work and reinforce protocol output - #7804

Merged
AmyShang-alt merged 6 commits into
mainfrom
feat/od-next-tool-call-prompt-optimization
Sep 8, 2026
Merged

perf(od-next): streamline tool work and reinforce protocol output#7804
AmyShang-alt merged 6 commits into
mainfrom
feat/od-next-tool-call-prompt-optimization

Conversation

@open-design-crew

@open-design-crew open-design-crew Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Why

OD Next evaluation runs were spending excessive tool actions on repeated edits, re-reads, and media capability probing. This PR brings the tool-work and media-input optimization into main together with the protocol-output follow-up from #7764: long production turns must still emit the closing Runtime State that the coordinator requires.

The final change:

  • Core: combine independent work, reuse complete current context, preserve every deliverable and quality requirement, and avoid wording that mixes protocol output with tool restrictions.
  • Orchestration: make source reads targeted, group compatible edits, and load od-next-media-inputs only when the task and stage need media work.
  • Media Skill: reuse capability evidence, classify failures before retries, batch independent acquisition, and stop polling terminal jobs.
  • Output contract and production continuation: require tagged JSON in the response body, restate the production Runtime State fields and terminal outcomes, and exclude Plan Contract output during production. Schema validation and coordinator acceptance remain unchanged.

Historical evidence (not a measurement of this updated main-based head): the original two-before/two-after, 15-case comparison reported 27.1 → 22.9 tool calls and 2.81M → 2.18M input tokens per question, while duration stayed approximately 11 minutes. Quality was not evaluated. See the evaluation report, initial analysis, and optimization proposal.

The follow-up #7764 recorded more protocol-blocked attempts after optimization, including both Plan Contract and Runtime State errors. The prompt changes address suspected instruction interference and missing production reminders; they do not establish a single cause for all failures or prove that model-level failure rates are fixed.

What users will see

Runs that select the OD Next strategy receive the optimized instructions and stronger production closing-state guidance. Media tasks can load the new OD Next Media Inputs utility through the existing od skill show path. No new UI, endpoint, or rollout default is introduced. Existing Skill CLI Workspace flags now reach the daemon; agent wrappers receive run-pinned OD_WORKSPACE_ID and OD_WORKSPACE_MEMBER_ID values. Existing frozen task prompts remain frozen; new tasks are needed to evaluate this version.

The strategy package becomes 2.0.4, Core remains 2.2.1, and orchestration becomes 2.0.2. Both daemon and API/BYOK OD Next composition use the shared contracts implementation; the ordinary strategy path is outside this PR.

Surface area

  • UI
  • Keyboard shortcut
  • CLI / env var — existing Skill list/show Workspace flags are forwarded; run-pinned OD_WORKSPACE_ID and OD_WORKSPACE_MEMBER_ID reach agent wrappers, including Codex shell filtering
  • API / contract — shared protocol instruction text changes; JSON schemas and HTTP shapes do not
  • Extension point — new media-input Skill and updated bundled strategy assets
  • i18n keys
  • New top-level dependency
  • Default behavior change — updated instructions for runs already routed to OD Next
  • None

UI/CLI parity: both entry points consume the same daemon strategy flow. This changes existing instruction content, not a new user-facing operation.

Screenshots

N/A — no UI changes.

Bug fix verification

The existing production continuation test was extended with #7764's closing-state assertions. Before applying the source fix, it failed specifically because ## Closing Runtime State was absent (23 passed / 1 failed); the pre-fix composition source was identical to the synced main version. The test proves instruction delivery, not model compliance.

The daemon integration test also checks that the actual production invocation receives the closing-state reminder while clarification remains incremental. No paid model evaluation or historical Run rerun was performed during this update; representative post-update model evaluation remains outstanding.

Validation of the protocol update (17823050)

  • Node 24.15.0 / Corepack pnpm 10.33.2; frozen-lockfile installation completed under Node 24.
  • pnpm guard — passed.
  • pnpm typecheck — passed across the workspace and root scripts.
  • pnpm --filter @open-design/contracts test — 55 files / 508 tests passed, including the previously failing closing-state assertion.
  • pnpm --filter @open-design/plugin-runtime test — 10 files / 72 tests passed.
  • Four focused daemon files (CLI Skill lookup, strategy package/recipe, exact runtime input) — 39 tests passed.
  • tests/od-next-automatic-simple-server.test.ts — 20 passed / 11 failed locally; failures are test/hook timeouts and database-closed errors after teardown, with no failed closing-state assertion. The full five-file daemon run took 610.71 s (59 passed / 11 failed).
  • Baseline comparison: a separate, unmodified worktree at main c5ae6292, with its own frozen-lockfile installation, reproduces the first failing runs parsed plan -> serialization repair -> production... case: test timeout at 20 s plus afterEach timeout at 10 s. This demonstrates that this failure also exists without the patch; it does not classify all other local failures or prove model compliance. The local automatic-production suite is not claimed green.
  • Remote CI for 17823050f9ee4d4e36cf070df6ec9d490bfa3af8 — completed: 24 successful checks, zero failed or pending checks; non-applicable jobs were skipped. Validate workspace and all four daemon shards passed. PR is mergeable, with review still required.

Main integration and conflict resolution

Synced main at c5ae6292c464094de80d0191fab50144bff8a027 with a merge commit, preserving published PR history. The only conflict was assets.core.version in the strategy manifest: main's 2.2.0 and new Skill read-only guidance were preserved, then the combined prompt was versioned as 2.2.1. Package version is 2.0.3; orchestration remains 2.0.1.

Only the scoped #7764 prompt changes and relevant tests were transferred. The test branch's Image routing/default activation and unrelated HomeHero assertions are not included. The final diff against main contains only the tool-work optimization, protocol instruction follow-up, version metadata, and their tests.

Workspace-scope review follow-up

Addresses AmyShang-alt's review. runLibraryList parsed Workspace flags for Skills but discarded them, so the new dynamic media Skill call could read the bundled body instead of the Workspace owner's same-id override.

  • Forward the existing Workspace/member flags on Skill list/show, preserving design-system behavior and rejecting incomplete pairs before any catalog request.
  • Pin the project creator member alongside the existing Run Workspace binding. Project Run creation is creator-gated; this reuses that identity rather than discovering an account-level default. Persist the pair with Run state and pass it to the final agent wrapper environment. Clear inherited values for unbound local runs. Old bound Runs lacking a member fail the scoped lookup rather than guessing another identity.
  • Preserve both variables through Codex's shell filter and pass them explicitly in the actual media Skill command. Scope changes invalidate reusable dynamic Skill context; failures do not authorize dropping scope.
  • Version the changed orchestration as 2.0.2 and the package as 2.0.4; Core remains 2.2.1. No schema/acceptance, rollout, Image routing, or ship-on-write policy changes.

Regression evidence

Before source changes on 17823050, eight assertions failed across five files, including missing headers, missing pinned environment/member fields, Codex filtering, and the real prompt wrapper returning built-in when user was required. The Skill CLI implementation was identical on the freshly fetched main. The real daemon test executes the POSIX command extracted from the shipped prompt, using opposite-value private versus built-in bodies; owner, other member, and unbound callers are checked. The POSIX-shell test is explicitly skipped on Windows; platform-independent CLI/header and runtime-environment tests remain applicable there.

Updated-head local validation:

  • Nine focused daemon files: 225 tests passed, including Skill headers/incomplete identity, real same-id override/fallback, Run creation/persistence, pinned member, Codex shell filtering, and strategy package/recipe.
  • Contracts: 508 tests passed; plugin runtime: 72 tests passed.
  • Frozen-lockfile installation, guard and full workspace typecheck passed. Final-SHA remote CI will be reported separately after pushing; the previous SHA's checks above are not evidence for this new commit.
  • Model evaluation and manual QA remain separate and have not been replaced by these deterministic checks.

Adjacent issue (not changed)

A full-catalog skill list --json piped into a child-process capture was truncated around 64 KB, producing invalid JSON. That stdout completion issue is outside this Workspace propagation fix. List forwarding is checked at the CLI request boundary; same-id resolution and body isolation are exercised through the real daemon detail route. No retries or timeout increases were used to hide this observation.

Main conflict resolution — 2026-09-08

Synced main d82385309f5f76466313b020ed8cd8e53e22938a into this PR with merge commit e1840142497a20b77351adb76fb1be01b3a39ae6, preserving published history. No force push or merge into main.

The single conflict was the production continuation payload in packages/contracts/src/prompts/od-next-strategy.ts:

  • Preserved main's hostProtocolKey and shared renderChatTurnHostProtocolInstructions(..., 'od_next_production') path, including keyed done, next-step and artifact-focus instructions and their stage gates. Main's runtime plan-tool note also remains intact.
  • Retained this PR's Closing Runtime State schema/terminal-outcome reminder and no-Plan-Contract production rule. Clarified that Runtime State precedes final host follow-up markers, so two protocols do not both demand the last output position; the general output contract excludes Plan Contract rather than excluding the host handoff markers.
  • Added composition assertions for coexistence/ordering guidance with a host key, and preservation of Closing Runtime State without injecting keyed markers when no key is supplied.

Audited the final diff against main: the original tool-work/Skill optimization, tool-name fix, Workspace/member propagation and protocol reminders remain; no unrelated main changes were dropped. The coordinator completion-fallback implementation is byte-identical to main. The product decisions discussed in the two QA threads remain unchanged: no post-first-write tool cutoff, no mandatory retry/rejection solely for an omitted Runtime State, and no new model evaluation claimed.

Validation on the merged tree (Node 24.15.0 / pnpm 10.33.2):

  • Frozen-lockfile installation completed; guard and full workspace/root typecheck passed.
  • Contracts: 63 files / 636 tests passed, including main's host-protocol coverage and the combined continuation assertions.
  • Plugin runtime: 10 files / 72 tests passed.
  • Focused daemon: 10 files / 273 passed, 1 skipped, including Workspace Skill lookup, runtime environment, persistence and coordinator completion fallback.
  • The focused real-server runs parsed plan -> serialization repair -> production after each source end test remains locally failing: 20 s test timeout plus 10 s teardown timeout. An independent unmodified worktree at the exact main SHA above, with its own frozen-lockfile installation, reproduces the same failures. This verifies the baseline failure, not a successful production flow; no timeout was raised and no assertion was removed.
  • git diff --check main...HEAD is clean. The lockfile is identical to the synced main; this PR adds no dependency or Nix hash change.

Remote CI for e18401424 is tracked separately from the previously green head; this section does not claim new-head CI or manual QA has passed.

bone3deep1962-collab and others added 2 commits September 6, 2026 12:14
The v2.1.1 core prompt named `open-design-runtime-state` inside the
"not a tool name" bullet. In the two post-optimization eval batches the
model called that exact name as a tool in 5 of 29 deliveries (1 of 30
before the sentence existed): a negated mention raises the name's
salience right where the model is about to emit the runtime-state block.

Keep the rule, drop the literal: protocol blocks are structured text
output, not tools. Bump core asset to 2.1.2 and the bundled package to
2.0.2; orchestration is untouched and stays 2.0.1.
@lefarcen lefarcen added risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps size/M PR changes 100-300 lines type/refactor Code refactor (no behavior change) labels Sep 6, 2026
@open-design-crew open-design-crew Bot changed the title perf(od-next): streamline tool use and media input prompts perf(od-next): streamline tool work and reinforce protocol output Sep 7, 2026
@lefarcen lefarcen added the needs-validation Runtime change detected; needs human or /explore agent validation. label Sep 7, 2026
@lefarcen

lefarcen commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Queued for QA validation — this PR changes runtime prompts and a media-input skill that need a manual QA pass before merge. Nothing needed from you; we will update here once it is validated.

@PerishCode PerishCode 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.

@app/open-design-crew I reviewed all changed ranges across the shared OD Next prompt composer, bundled strategy assets and version metadata, the new media-input skill, and the daemon/contracts/plugin-runtime tests. The production continuation now restates the strict terminal Runtime State shape without reintroducing a Plan Contract, the dynamic skill lookup uses the existing CLI seam, and the asset/package version updates are consistent with the changed prompt contents. Remote CI is green across the relevant workspace, daemon, E2E Vitest, and UI lanes. Nice work keeping this prompt optimization scoped and carrying the protocol-output follow-up through the real production invocation path.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@AmyShang-alt AmyShang-alt 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.

@app/open-design-crew Thanks for the contribution. Requesting changes because the new dynamic media Skill lookup loses the selected Workspace identity and can load the wrong same-id Skill body.

Comment thread plugins/_official/scenarios/od-next-strategy/assets/general-orchestration.md Outdated

@PerishCode PerishCode 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.

@app/open-design-crew I reviewed every changed range across the OD Next prompt contract and bundled assets, the media-input skill, and the daemon workspace-scope follow-up. The production continuation now carries the required terminal Runtime State reminder, while dynamic Skill reads preserve the run-pinned Workspace/member identity through persistence, runtime environment shaping, Codex filtering, and CLI headers; incomplete historical scope fails closed. I also verified the final head with 216 focused daemon tests, 508 contracts tests, and 72 plugin-runtime tests, alongside the green live CI suite. Thank you for responding thoroughly to the earlier scope issue and backing the fix with end-to-end negative-control coverage.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Comment thread packages/contracts/src/prompts/od-next-strategy.ts Outdated
@open-design-crew open-design-crew Bot added validated Runtime change validated (via /explore Pass or manual QA). and removed needs-validation Runtime change detected; needs human or /explore agent validation. labels Sep 7, 2026
@open-design-crew

Copy link
Copy Markdown
Contributor Author

@open-design-crew

Thanks for the contribution. I completed QA validation for this PR.

QA Acceptance Record

Scope:

  • OD Next tool-work/media prompt changes, production closing guidance, and the Workspace-scoped dynamic Skill lookup on head d1d805c.

Scope-to-evidence:

  • Workspace authority propagation -> 245 focused local tests passed, including owner/other-member/unbound negative controls, incomplete historical scope, persistence, runtime env, and Codex filtering; a fresh Electron run in Eli's workspace preserved the exact Workspace/member binding through Skill lookup and production.
  • Runtime delivery -> a fresh Electron Full Plan run produced a valid root index.html plus a generated local image; both runtime URLs returned 200 and the 375 px layout had no horizontal overflow.
  • CI -> exact head completed with 24 successful checks, zero failed or pending checks (6 non-applicable checks skipped).
  • Restoration -> PR-owned Electron/web/daemon services, ports, worktree, and tools root were removed; pre-existing Codex config/auth hashes and main-worktree changes remained unchanged.

Risks / notes:

  • Prompt adherence remains best-effort. Post-write activity and an omitted closing Runtime State were observed; the contributor explicitly documented the product decision to retain prompt-level guidance and the existing unambiguous Simple Full Plan completion fallback. Acceptance does not claim reliable model compliance or a representative multi-run performance improvement.
  • No analytics or UI contract changed; recorder/package validation was not required for this prompt/runtime-only scope.

Conclusion:

  • Accepted for head d1d805c with the documented product limitations above.

Preserve main's keyed host handoff protocols alongside the closing Runtime State reminder. Clarify marker ordering and cover keyed and unkeyed continuation composition. Keep existing completion fallback unchanged.

@PerishCode PerishCode 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.

@app/open-design-crew I reviewed all changed ranges on the merged head across the daemon CLI/runtime scope propagation, shared OD Next prompt composition, bundled strategy assets and versions, the new media-input skill, and their focused tests. The earlier workspace-scope blocker is closed by pinning and persisting the project owner member, forwarding the exact pair through agent environments and Skill list/show requests, clearing unbound ambient identity, and failing closed for incomplete historical scope. The production Runtime State guidance also coexists with main’s keyed host protocol in the intended order, with package and asset versions aligned. Thank you for the careful follow-up and the strong owner/other-member/unbound negative-control coverage.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@AmyShang-alt
AmyShang-alt added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit c8f6db6 Sep 8, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps size/M PR changes 100-300 lines type/refactor Code refactor (no behavior change) validated Runtime change validated (via /explore Pass or manual QA).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants