feat(harness): schema loader for Centaur agent definitions

[dimakis]

Summary

• Adds schema loader that bridges Centaur YAML agent definitions to Mitzo's DeliberationConfig / FusionConfig types
• buildDeliberationConfig() — takes proposer + challenger pair, validates roles/counterparts, applies defaults
• buildDeliberationConfigFromAgent() — async resolver-based: pass one agent, it looks up the counterpart
• buildFusionConfig() — model name list → panel/judge/synthesizer config
• 17 tests covering happy paths, defaults, overrides, and all error cases

Follows up on #387 (reasoning engine). Prerequisite for wiring ContexGin agent definitions into deliberation sessions.

Test plan

• 17 unit tests passing (vitest run packages/harness/__tests__/schema-loader.test.ts)
• tsc -b clean
• Lint + prettier passing (pre-commit hooks)
• CI green
• Centaur review

🤖 Generated with Claude Code

[dimakis]

Centaur Review

Found 6 issue(s) (1 warning).

packages/harness/__tests__/schema-loader.test.ts

Clean, well-structured addition with good test coverage for the happy paths and core error cases. The main substantive issue is that the challenger's protocol settings are silently dropped when the proposer has none — a one-line fallback fix. The remaining items are minor test gaps and a misleading JSDoc comment.

• 🔵 missing_tests: No test for counterpart mismatch warnings. buildDeliberationConfig warns when proposerDef.deliberation.counterpart !== challengerDef.identity.name (and vice versa), but this path has no test coverage. A test with mismatched counterpart names would verify the function still succeeds (doesn't throw) while logging the warning. [fixable]
• 🔵 missing_tests: No test for buildDeliberationConfigFromAgent when the resolver throws (e.g. counterpart not found). The function doesn't catch resolver errors, so the test should verify the rejection propagates cleanly. This is a likely real-world failure mode. [fixable]
• 🔵 missing_tests: No test for buildDeliberationConfigFromAgent when the resolved counterpart has the wrong role (e.g. resolver returns another proposer instead of a challenger). This would exercise the validation in buildDeliberationConfig through the convenience path. [fixable]
• 🔵 missing_tests: buildFusionConfig accepts a judgeModel string that doesn't need to be in the panelModels list. There's no test verifying behavior when judgeModel is a model not present in the panel — this may be intentional but worth documenting with a test. [fixable]

packages/harness/src/reasoning/schema-loader.ts

Clean, well-structured addition with good test coverage for the happy paths and core error cases. The main substantive issue is that the challenger's protocol settings are silently dropped when the proposer has none — a one-line fallback fix. The remaining items are minor test gaps and a misleading JSDoc comment.

• 🟡 bugs (L121): Protocol settings are only read from the proposer's definition (proposerDef.deliberation.protocol). When buildDeliberationConfigFromAgent is called with a challenger that has protocol settings but its resolved proposer counterpart does not, the challenger's protocol settings are silently ignored and defaults are used instead. The docstring says 'proposer's take precedence' but it would be more robust to fall back to the challenger's protocol when the proposer's is absent: const protocol = proposerDef.deliberation.protocol ?? challengerDef.deliberation.protocol ?? {}; [fixable]
• 🔵 style (L191): JSDoc comment says synthesizerModel 'defaults to judge model' but the implementation leaves it undefined when not specified. The FusionOrchestrator may handle this fallback internally, but the JSDoc on BuildFusionConfigOptions is misleading about where the default is applied. [fixable]

[dimakis]

🟡 bugs: Protocol settings are only read from the proposer's definition (proposerDef.deliberation.protocol). When buildDeliberationConfigFromAgent is called with a challenger that has protocol settings but its resolved proposer counterpart does not, the challenger's protocol settings are silently ignored and defaults are used instead. The docstring says 'proposer's take precedence' but it would be more robust to fall back to the challenger's protocol when the proposer's is absent: const protocol = proposerDef.deliberation.protocol ?? challengerDef.deliberation.protocol ?? {}; [fixable]

[dimakis]

🔵 style: JSDoc comment says synthesizerModel 'defaults to judge model' but the implementation leaves it undefined when not specified. The FusionOrchestrator may handle this fallback internally, but the JSDoc on BuildFusionConfigOptions is misleading about where the default is applied. [fixable]

[dimakis]

Centaur Review

Found 2 issue(s).

packages/harness/src/reasoning/schema-loader.ts

Clean, well-tested feature. Schema-loader logic is correct and thoroughly exercised; only minor suggestions around an unused type field and an unrelated formatting change.

• 🔵 unsafe_assumptions (L32): The phases field in AgentDeliberationBlock.protocol is accepted in the type but never read or validated by buildDeliberationConfig. Agent definitions specifying custom phases (e.g. omitting 'converge') will be silently ignored since the orchestrator hardcodes a fixed 4-phase protocol. Consider either consuming and validating this field or dropping it from the type to avoid misleading callers. [fixable]

packages/protocol/src/tool-summary.ts

Clean, well-tested feature. Schema-loader logic is correct and thoroughly exercised; only minor suggestions around an unused type field and an unrelated formatting change.

• 🔵 style (L94): This is a formatting-only change (line wrapping) unrelated to the schema-loader feature. Ideally it would be a separate commit to keep the PR diff focused.

[dimakis]

🔵 unsafe_assumptions: The phases field in AgentDeliberationBlock.protocol is accepted in the type but never read or validated by buildDeliberationConfig. Agent definitions specifying custom phases (e.g. omitting 'converge') will be silently ignored since the orchestrator hardcodes a fixed 4-phase protocol. Consider either consuming and validating this field or dropping it from the type to avoid misleading callers. [fixable]

[dimakis]

🔵 style: This is a formatting-only change (line wrapping) unrelated to the schema-loader feature. Ideally it would be a separate commit to keep the PR diff focused.

[dimakis]

Centaur Review

Found 4 issue(s).

packages/harness/src/reasoning/schema-loader.ts

Clean, well-tested schema loader with thorough test coverage. All findings are minor style/design suggestions — no bugs or regressions.

• 🔵 style (L220): Fusion default budget (3.0) is hardcoded inline, while deliberation uses a named constant (DEFAULT_BUDGET_USD = 2.0). For consistency, extract a DEFAULT_FUSION_BUDGET_USD = 3.0 constant. [fixable]
• 🔵 unsafe_assumptions (L134): Temperature is always set (0.7 proposer, 0.8 challenger) even though AgentRole.temperature is optional. This means callers cannot opt out of setting a temperature through the builder to let the provider use its native default. Consider only setting temperature when explicitly provided in options. [fixable]

packages/harness/src/index.ts

Clean, well-tested schema loader with thorough test coverage. All findings are minor style/design suggestions — no bugs or regressions.

• 🔵 style (L99): Three separate export { ... } from './reasoning/index.js' blocks (lines 99, 100-104, 105-109) could be consolidated into a single export block for readability. [fixable]

packages/harness/__tests__/schema-loader.test.ts

Clean, well-tested schema loader with thorough test coverage. All findings are minor style/design suggestions — no bugs or regressions.

• 🔵 missing_tests (L155): The counterpart mismatch test verifies the function doesn't throw, but doesn't assert that a warning was logged. Consider spying on the logger to confirm log.warn is called with the expected mismatch details. [fixable]

[dimakis]

Centaur Review

Found 3 issue(s).

packages/harness/src/index.ts

Clean, well-tested schema loader — good validation, correct type narrowing via Omit<..., 'onEvent'>, thorough test coverage. Only minor style and test symmetry suggestions.

• 🔵 style (L105): Two separate export { ... } from './reasoning/index.js' blocks (lines 100-104 and 105-109) could be consolidated into one. The type exports are already merged into a single block. [fixable]

packages/harness/__tests__/schema-loader.test.ts

Clean, well-tested schema loader — good validation, correct type narrowing via Omit<..., 'onEvent'>, thorough test coverage. Only minor style and test symmetry suggestions.

• 🔵 missing_tests: The 'wrong role' test for buildDeliberationConfigFromAgent only covers the proposer-input case (proposer resolves another proposer). The symmetric case — challenger input whose resolver returns another challenger — exercises a different code path (the else branch swaps args) and would catch a swap bug that the current test wouldn't. Consider adding a test for it. [fixable]

packages/harness/src/reasoning/schema-loader.ts

Clean, well-tested schema loader — good validation, correct type narrowing via Omit<..., 'onEvent'>, thorough test coverage. Only minor style and test symmetry suggestions.

• 🔵 style (L1): The 17-line module-level JSDoc is useful documentation but conflicts with the project's code style guideline ('never write multi-paragraph docstrings or multi-line comment blocks — one short line max'). Consider shortening to a one-liner or moving the YAML example to a doc or README. [fixable]

[dimakis]

🔵 style: Two separate export { ... } from './reasoning/index.js' blocks (lines 100-104 and 105-109) could be consolidated into one. The type exports are already merged into a single block. [fixable]

[dimakis]

🔵 style: The 17-line module-level JSDoc is useful documentation but conflicts with the project's code style guideline ('never write multi-paragraph docstrings or multi-line comment blocks — one short line max'). Consider shortening to a one-liner or moving the YAML example to a doc or README. [fixable]