Skip to content

Add routing guide and subagent thinking overrides - #405

Open
m-aebrer wants to merge 5 commits into
masterfrom
feature/issue-394-routing-guide-thinking
Open

Add routing guide and subagent thinking overrides#405
m-aebrer wants to merge 5 commits into
masterfrom
feature/issue-394-routing-guide-thinking

Conversation

@m-aebrer

@m-aebrer m-aebrer commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Part of #394

Stage 1 adds the model-routing-guide research skill and explicit per-subagent thinking overrides with effective thinking observability.

Implementation plan posted as a comment below.

@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Implementation Plan — Stage 1

Problem and approach

Stage 1 has two focused deliverables:

  1. A built-in model-routing-guide skill that uses normal dreb tools to research the explicitly configured model set, mine existing subagent sessions, and write a durable guide for the later arbiter.
  2. An optional thinking override on the existing subagent tool, with strict validation and effective-level metadata.

Per the user mandate, guide generation stays entirely in SKILL.md. This PR will not add a routing runtime, settings API, parser service, benchmark framework, or arbiter code. The skill will read the existing settings/model/session files and use normal shell/read/web tools. The guide's most important recommendations will support the later arbiter in catching role mistakes such as planning or implementation work delegated to Explore, and in routing mundane lookup/file-inspection work away from expensive frontier models.

External prior art supports this narrow design:

  • RouteLLM frames routing as sending work a weaker model can handle to that model while reserving the stronger model for work that needs it, optimizing quality against cost.
  • OpenAI's evaluation guidance recommends task-specific evaluation, mining historical logs, combining metrics with judgment, and explicitly evaluating agent tool selection and multi-agent handoff accuracy. Those map directly to local subagent evidence and the Explore misuse cases.

Deliverables

1. Built-in model-routing-guide skill

Create packages/coding-agent/skills/model-routing-guide/SKILL.md as an explicitly user-invoked research workflow.

The skill will:

  • Determine the model patterns from explicit skill arguments when supplied, otherwise from the effective readable enabledModels settings; refuse when neither provides a non-empty scope.
  • Resolve patterns to canonical provider/model candidates using the normal installed CLI/model listings, and refuse an effectively all-model scope rather than launching unbounded research.
  • Snapshot and inspect existing files under the subagent-session directory before doing research, so sessions created by the guide run do not contaminate its own evidence set.
  • Fail loudly when existing session files cannot be read or parsed; use an explicit cold-start mode only when no prior child sessions exist.
  • Analyze requested agent role, task category, canonical model/provider, effective thinking level, tools used, completion/failure/truncation/retry/cancellation signals, semantic task satisfaction, and parent-linked corrections or repeated delegations where available.
  • Aggregate local findings by canonical model, agent role, task category, and thinking level with sample counts and confidence.
  • Research provider/model documentation, model cards, benchmarks/leaderboards, issue trackers, Reddit/forums, and practitioner reports, keeping vendor claims, measured benchmarks, community reports, and local observations visibly separate.
  • Give concrete role and cost guidance, including two explicit guardrails: Explore is for factual collection/navigation rather than planning or feature implementation, and routine high-volume lookup/file work should use the least expensive scoped model shown by evidence to be adequate.
  • Write ~/.dreb/agent/model-routing-guide.md with stable YAML frontmatter (schema_version, generation date, canonical covered IDs, cold-start/local-evidence status, analyzed directories/date range) and one consistently structured section per canonical candidate.
  • Include strengths, weaknesses, failure modes, recommended and discouraged roles, thinking support, context/vision/tool behavior, latency/cost observations, confidence, contrary evidence, dated source URLs, and explicit unknowns rather than inventing missing evidence.
  • Never copy prompts, outputs, tool arguments, secrets, project names, paths, or other confidential session content into the guide; only generalized categories and aggregate findings may be written.
  • Re-read the generated file and perform a final procedural validation that frontmatter and model sections cover the exact resolved scope and contain every required field; report failure rather than claiming success if coverage or schema checks fail.

No helper executable or production TypeScript is part of guide generation in Stage 1.

2. Subagent thinking override

Extend the existing request path in packages/coding-agent/src/core/tools/subagent.ts:

  • Add thinking to single, parallel-item, and chain-step schemas using the existing six-level enum.
  • Apply precedence per-task thinking > top-level thinking > omitted for parallel and chain modes.
  • Thread an explicit value through executeSingle() to spawnSubagent() and pass it as --thinking only when supplied. Omission must produce no child argument and preserve today's child/settings behavior.
  • Validate only after the final child model resolves. Explicit non-off thinking on a non-reasoning model and explicit xhigh on a model without xhigh support must fail before spawn rather than relying on the child's normal silent capability clamp. Reasoning models otherwise support the standard minimal through high levels.
  • Keep validation reusable in packages/coding-agent/src/core/thinking.ts so Stage 2 uses the same capability rule.

3. Effective thinking metadata

Expose what the child actually uses, including when no override was supplied:

  • Extend the core agent_start event in packages/agent/src/types.ts and packages/agent/src/agent-loop.ts with the effective thinking level alongside canonical model identity.
  • Capture that field from the child JSON event stream in subagent.ts and add it to SubagentResult.
  • Include model/thinking in formatted subagent results and the typed background_agent_end event emitted by AgentSession, without injecting raw internal data into child or parent prompts.
  • Preserve current event compatibility by making the additional metadata additive.

The existing background_agent_event relay then carries effective model/thinking to JSON mode, RPC, and dashboard transports without a new Stage 1 event family.

Files to create

  • packages/coding-agent/skills/model-routing-guide/SKILL.md — complete skill-only research, privacy, output, and validation workflow
  • packages/coding-agent/test/subagent-thinking-override.test.ts — focused schema, precedence, capability, child-argument, result-capture, and omission tests

Files to modify

  • packages/coding-agent/src/core/tools/subagent.ts — schema, precedence, validation call, child args, JSON event capture, result metadata/formatting
  • packages/coding-agent/src/core/thinking.ts — shared explicit-level capability validator
  • packages/coding-agent/src/core/agent-session.ts — typed completion event metadata and parent-facing result summary
  • packages/agent/src/types.ts — additive agent_start.thinkingLevel
  • packages/agent/src/agent-loop.ts — emit the effective level
  • packages/agent/test/agent-loop.test.ts — core event payload coverage
  • packages/coding-agent/test/thinking.test.ts — reasoning, non-reasoning, and xhigh validation cases
  • packages/coding-agent/test/skills.test.ts — built-in discovery and mandatory skill-contract instructions
  • packages/coding-agent/test/agent-session-guardrails.test.ts — completion event/result metadata coverage
  • README.md — public mention of evidence-based routing guidance and per-child thinking control
  • packages/coding-agent/README.md — subagent parameter/precedence/metadata and built-in skill usage
  • packages/agent/README.md — additive agent_start payload documentation
  • packages/coding-agent/docs/skills.md — built-in skill invocation, generated guide, evidence/privacy behavior
  • packages/coding-agent/docs/agent-models.md — relationship between scoped models, guide research, model precedence, and thinking overrides
  • packages/coding-agent/docs/rpc.md — effective thinking fields on relayed start/completion events

If implementation shows an already-existing test file is the clearer home for a case, tests may be consolidated there rather than duplicated, but every behavior listed below remains required.

Acceptance criteria

Skill

  • The built-in skill is discoverable and user-invocable without adding runtime guide infrastructure.
  • Missing/empty and effectively all-model scopes stop loudly before research.
  • Every resolved canonical candidate gets one complete guide entry and appears in frontmatter; post-write coverage validation is mandatory.
  • Existing readable child history is analyzed; unreadable/malformed existing logs stop generation; no-history runs are marked cold-start.
  • Local results include semantic outcome assessment and grouping by model, role, task category, and thinking with sample counts/confidence.
  • External and local evidence are separately labeled, dated, cited, reconciled, and allowed to disagree.
  • The guide contains only sanitized generalized findings, never copied session content or project identifiers.
  • Role recommendations explicitly reject Explore for planning/implementation and discourage frontier-model use for routine bulk lookup/file work unless evidence requires it.

Thinking override

  • All six supported values are accepted in single, parallel, and chain requests; other strings are schema errors.
  • Per-task values override top-level values.
  • Valid explicit levels produce exactly one --thinking <level> child argument.
  • Unsupported explicit levels fail before spawn with an actionable error.
  • Omission adds no argument and preserves existing behavior.
  • SubagentResult, agent_start, formatted results, and background_agent_end report the actual effective level.

Testing approach

  • Skill contract tests: load the real built-in skill and assert explicit invocation policy plus mandatory scope gates, local-log failure/cold-start rules, evidence categories, confidentiality prohibition, exact-coverage validation, and the two user routing priorities.
  • Schema and precedence tests: exercise the real subagent tool schema and launch path for all three modes, including valid/invalid values and top-level versus item-level selection.
  • Capability tests: cover off on any model, normal levels on reasoning models, non-off rejection on non-reasoning models, and xhigh support/rejection.
  • Spawn tests: use the existing mocked child-process pattern to inspect arguments, verify omission, and feed synthetic agent_start JSON carrying effective thinking.
  • Event/result tests: verify additive core event typing, result formatting, parent completion delivery, and typed background completion metadata.
  • Regression: run targeted Vitest files, npm test, npm run build, and npm run verify-workspace-links; after the required build, manually exercise supported, unsupported, inherited, and omitted thinking through the real dreb -p binary.
  • Skill QA: explicitly invoke the built-in skill against controlled no-scope, cold-start, and small sanitized-history setups and inspect refusal/output/coverage behavior without using live API calls in automated tests.

Risks and boundaries

  • The skill necessarily asks the active research model to inspect historical local content. Documentation and skill instructions must state that clearly and prohibit verbatim guide output; users control when the expensive workflow runs.
  • External evidence can be sparse or contradictory for custom provider/model pairs. The guide must record low confidence and unknowns instead of collapsing providers or fabricating conclusions.
  • Adding thinking to a core event touches all transports, so event payload tests must guard backward-compatible additive behavior.
  • Guide parsing by the arbiter, arbiter settings/model calls, pre-spawn decision logic, arbitration events, and dashboard decision UI belong to Stage 2 and are intentionally excluded here.

No Stage 1 design question remains blocking after the user's simplification mandate: guide generation is a skill workflow, while production code changes are limited to the requested thinking override and effective metadata.


Plan created by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Progress Update

Implemented Stage 1:

  • Added the explicitly invoked model-routing-guide skill as a pure SKILL.md workflow. It gates missing/all-model scopes, requires readable existing child logs or marks true cold-start, separates vendor/benchmark/community/local evidence, protects persisted-guide confidentiality, encodes the Explore-role and frontier-cost safeguards, and validates exact guide coverage after writing.
  • Added optional thinking overrides for single, parallel, and chain subagents with per-task precedence and strict resolved-model capability validation.
  • Passed supported overrides to children through --thinking; omission preserves child defaults.
  • Added effective thinking metadata to core agent_start, child event capture, subagent results, parent completion messages, and background_agent_end.
  • Updated root, coding-agent, agent-core, skills, agent-model, JSON, and RPC documentation.
  • Added focused schema, precedence, spawn-argument, capability, event, formatting, skill-contract, and transport tests.

Verification:

  • npm run build passed.
  • Commit hook passed 5,118 tests with 709 live tests skipped.
  • npm run verify-workspace-links passed.
  • Real binary QA confirmed a supported off override, loud rejection of unsupported xhigh before child spawn, effective thinking in child/result events, and all-model guide refusal without modifying the guide.
  • A separate authenticated live-suite attempt reached the external Codex usage limit; the complete non-live suite passed.

Commit: 168673e


Progress tracked by mach6

@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown

Vitest coverage

Metric Covered Total Coverage
Statements 25917 43041 60.21%
Branches 14038 26477 53.01%
Functions 4912 7826 62.76%
Lines 22128 37066 59.69%

View full coverage run

@m-aebrer
m-aebrer marked this pull request as ready for review July 27, 2026 20:29
@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Code Review

Critical

None.

Important

Finding 1 — Fallback-selected model capability validation lacks an integration test

Severity: high · Confidence: 97

packages/coding-agent/test/subagent-thinking-override.test.ts tests fallback selection and strict thinking validation separately, but never combines them. It does not prove that an override is validated against the final model selected after availability probing. Add cases where an unavailable reasoning primary falls back to (a) a non-reasoning model that must reject non-off thinking before spawn and (b) a reasoning model that must spawn once with the override.

Suggestions

Finding 2 — Effective-thinking telemetry test does not distinguish child-reported metadata from the request

Severity: medium · Confidence: 98

The explicit-level test requests high and makes the child emit high, so it would pass if SubagentResult.thinking merely echoed the override. Add a background-path case that passes --thinking high, emits child thinkingLevel: "low", and verifies the result, parent completion, and background_agent_end all report low.

Finding 3 — Continued-loop and default-off lifecycle metadata are untested

Severity: low · Confidence: 99

runAgentLoopContinue() now emits the new field, but its test does not assert the agent_start payload. No test protects config.reasoning ?? "off". Add coverage for a configured continuation level and omitted-reasoning off.

Finding 4 — Model-inheriting agents reject valid non-off overrides without an actionable remedy

Severity: low · Confidence: 82

For an agent with no model field, no invocation override, and no agentModels entry, resolvedModel remains undefined. Every explicit non-off level then fails before spawn with “no concrete child model was resolved,” although the child would inherit its runtime model. Either resolve and validate the inherited parent model, or make the error explicitly tell the caller to set an agent/per-call model so validation can occur.

Finding 5 — Subagent result metadata drops the resolved provider

Severity: medium · Confidence: 88

agent_start carries { provider, id }, but handleChildJsonlLine() passes only event.model.id to SubagentResult.model; background_agent_end forwards that ambiguous value. Preserve canonical provider/model identity so proxy, cloud, and direct-provider variants cannot collapse to the same model ID.

Finding 6 — Chain completion events omit effective model/thinking metadata

Severity: medium · Confidence: 92

Chain steps retain metadata in their formatted results, but the aggregate SubagentResult returned to _handleBackgroundComplete() has neither scalar metadata nor structured per-step metadata. Consequently chain background_agent_end always omits these fields. Add explicit per-step completion metadata, or another unambiguous representation suitable for heterogeneous chains.

Finding 7 — Thinking-level schema literals are duplicated

Severity: medium · Confidence: 95

thinkingLevelSchema is used for task items, while the top-level schema manually repeats the same six literals. Reuse one schema definition while preserving the top-level description if needed.

Finding 8 — Result metadata formatting is duplicated

Severity: low · Confidence: 90

agent-session.ts and subagent.ts independently build the same model/thinking string. A shared helper colocated with SubagentResult would keep formatting consistent.

Finding 9 — Thinking precedence helper wraps only nullish coalescing

Severity: low · Confidence: 82

resolveSubagentThinkingOverride() is an exported wrapper around taskThinking ?? topLevelThinking with dedicated tests. Consider inlining the expression at its two call sites; retain the helper only if the named policy boundary is intentional.

Strengths

  • The skill remains a focused, user-invoked SKILL.md workflow as authorized, with strong scope, local-evidence, privacy, source-labeling, cold-start, and exact-coverage instructions.
  • Schema support, per-task precedence, strict non-reasoning/xhigh rejection, child argument threading, omission behavior, and additive lifecycle metadata are cleanly integrated.
  • The child JSONL parser validates thinking values without dropping relayed events.
  • Completeness review found every authoritative Stage 1 acceptance criterion and planned documentation surface represented.
  • Targeted reviewer verification passed 231 tests across seven relevant files.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier


Reviewed by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Review Assessment

Review comment

Classifications

Finding Classification Reasoning
Finding 1 — Fallback-selected capability validation test genuine Factual: Fallback selection and thinking rejection are tested separately; no case proves validation uses the final probed fallback. Scope: The approved plan requires post-resolution validation and pre-spawn capability tests for this new interaction.
Finding 2 — Child-effective telemetry precedence test genuine Factual: The explicit test requests and emits high, so it cannot catch reversing resolvedThinking ?? thinkingOverride; parent completion is tested with a fabricated result. Scope: Stage 1 requires the actual child-effective level in results and completion events.
Finding 3 — Continued-loop/default-off event tests genuine Factual: Both loop entry points changed, but only normal-loop explicit high is asserted; continuation and the off branch are uncovered. Scope: The plan requires additive core event coverage and effective metadata when no override is supplied.
Finding 4 — Inherited-model override validation genuine Factual: A model-less agent leaves resolvedModel undefined despite available parent model/provider, causing every non-off override to fail with no configuration remedy. Scope: Parent-model inheritance is part of routing precedence, and validation must target the final child model with actionable failures.
Finding 5 — Provider omitted from result model genuine Factual: Child agent_start has provider and ID, but result extraction stores only the ID, making completion telemetry ambiguous. Scope: Stage 1 requires actual effective model observability and treats canonical provider/model identity as significant.
Finding 6 — Chain completion metadata omitted genuine Factual: Per-step formatted output has metadata, but the aggregate result has no model/thinking, so chain background_agent_end omits both. Scope: Effective observability and thinking overrides apply to chain mode; heterogeneous chains require structured per-step metadata.
Finding 7 — Duplicated thinking schema nitpick Factual: The same valid six-literal union is declared twice. Scope: Reuse is maintainability-only and fixes no current requirement or behavior.
Finding 8 — Duplicated metadata formatting nitpick Factual: Two call sites build the same inner model/thinking string and currently agree. Scope: A helper would be refactoring only.
Finding 9 — Nullish-coalescing precedence helper nitpick Factual: The helper wraps one ?? expression but also names and tests the policy. Scope: Inlining changes no behavior or acceptance criterion.

Action Plan

  1. Resolve model-inheriting agents to the concrete parent/effective model before validating explicit thinking, with actionable errors if resolution is impossible.
  2. Preserve canonical provider/model identity from child agent_start through SubagentResult, formatted output, and background_agent_end.
  3. Add structured per-step effective model/thinking metadata to chain completion results and events.
  4. Add fallback integration tests for non-reasoning rejection and reasoning success after final fallback selection.
  5. Add a background-path test where requested and child-reported thinking differ, verifying child-effective metadata through result delivery and completion events.
  6. Add agent_start tests for configured continuation thinking and omitted-reasoning off.

Assessment by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Progress Update

Fixed genuine review findings 1–6:

  • Added integration coverage proving thinking validation uses the final model selected after fallback probing.
  • Resolved model-less agents to the inherited parent model before strict thinking validation, with an actionable error when no model is available.
  • Preserved canonical provider/model identity from child lifecycle events through subagent results and background completion events.
  • Added ordered per-step model/thinking metadata for heterogeneous chain completion results and events.
  • Added requested-versus-child-effective telemetry precedence coverage through the background path.
  • Covered configured continuation thinking and the default off lifecycle payload.
  • Updated coding-agent README and JSON/RPC event documentation.

Verification:

  • Commit hook passed 5,126 tests with 709 live tests skipped.
  • Targeted suite passed 175 tests.
  • Full non-live workspace suite passed.
  • npm run build passed.
  • npm run verify-workspace-links passed.
  • Real built-binary QA confirmed that a model-less custom agent inherited github-copilot/gpt-5.6-sol, ran at thinking: low, and emitted canonical model/thinking metadata.
  • An unrestricted live-suite attempt reached the external OpenAI Codex usage limit; the complete non-live suite passed.

Commit: 06854cb


Progress tracked by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Code Review

Critical

None.

Important

None.

Suggestions

Finding 1 — Effective thinking can report requested xhigh instead of the provider-clamped level

Severity: medium · Confidence: 85

Both entry points in packages/agent/src/agent-loop.ts emit config.reasoning ?? "off". For a reasoning model without xhigh support, provider adapters can clamp a normal/default session request from xhigh to high, while the lifecycle event still reports xhigh. Because subagent result telemetry trusts that event, formatted results, completion events, and future routing evidence can misstate the effective level. Centralize capability-aware effective-level resolution before emitting agent_start, while retaining strict rejection for explicit subagent overrides.

Finding 2 — Registry-less integrations reject every explicit non-off thinking level with a misleading error

Severity: low · Confidence: 83

packages/coding-agent/src/core/tools/subagent.ts permits construction without a modelRegistry. Model-string resolution can still succeed in that mode, but no concrete Model object is available, so validateThinkingLevelForModel(undefined, level) rejects every non-off override as though no model resolved. Clarify and intentionally define registry-less behavior: either require a registry for strict capability validation with an accurate actionable error, or otherwise reconcile this embedding path with the fail-loudly validation contract.

Finding 3 — Four supported levels lack child-launch argument coverage

Severity: medium · Confidence: 99

packages/coding-agent/test/subagent-thinking-override.test.ts schema-checks all six literals, but spawn-path tests exercise only low and high. Add launch-path assertions for explicit off, minimal, medium, and supported xhigh, especially proving that explicit off emits exactly --thinking off rather than behaving like omission.

Finding 4 — Child-effective metadata is not tested end-to-end through parent and chain completion observability

Severity: medium · Confidence: 98

The child-process test proves a requested high can produce SubagentResult.thinking: low, while parent completion tests use fabricated results and chain tests request the same values the child emits. Add integrated background and chain cases where child agent_start reports canonical provider/model and thinking values deliberately different from the request, then verify parent delivery and background_agent_end.steps preserve those child-reported values.

Finding 5 — Skill contract tests weakly protect malformed-history refusal

Severity: medium · Confidence: 97

packages/coding-agent/test/skills.test.ts checks only the broad phrase that snapshotted files are required evidence. It would not fail if the skill lost its explicit JSONL parse gate, loud unreadable/malformed-file refusal, affected-file reporting, or prohibition against relabeling broken history as cold-start. Assert those mandatory instructions directly.

Finding 6 — Thinking-level schema literals are duplicated

Severity: medium · Confidence: 95

packages/coding-agent/src/core/tools/subagent.ts defines thinkingLevelSchema for task items but repeats the same six-literal union for the top-level field solely to use a different description. Reuse one literal source while preserving field-specific descriptions so the accepted values cannot drift.

Finding 7 — Result metadata formatting is duplicated

Severity: low · Confidence: 90

subagent.ts and agent-session.ts independently build the same model/thinking metadata string. A shared formatter colocated with SubagentResult would keep future fields consistent. This is an optional refactor.

Finding 8 — Thinking precedence helper wraps only nullish coalescing

Severity: low · Confidence: 85

resolveSubagentThinkingOverride() is an exported wrapper around taskThinking ?? topLevelThinking with dedicated tests. Consider inlining it unless the named policy boundary is intentionally retained for readability or expected future logic.

Finding 9 — The model-less guard partially duplicates validator behavior

Severity: medium · Confidence: 82

In executeSingle(), a dedicated !modelSpec guard precedes validateThinkingLevelForModel(resolvedModel, thinkingOverride), which also rejects undefined models. Either collapse the duplicated branches or document that the first exists to provide the richer agent-specific remediation message.

Strengths

  • Completeness review found every authoritative Stage 1 acceptance criterion, planned documentation surface, and user routing mandate implemented without leaking Stage 2 scope.
  • Final-fallback and inherited-parent model resolution now feed strict thinking validation before spawn.
  • Canonical provider/model identity and ordered heterogeneous chain metadata flow through results and completion events.
  • Omission preserves child defaults, while explicit values are passed only through --thinking.
  • The skill remains a focused, explicitly invoked SKILL.md workflow with strong scope, evidence, confidentiality, cold-start, and exact-coverage instructions.
  • Error-path review found no silent data corruption or swallowed errors.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier


Reviewed by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Review Assessment

Review comment

Classifications

Finding Classification Reasoning
Finding 1 — Effective thinking can report unclamped xhigh genuine Factual: Both loop entry points emit the configured level, while provider adapters clamp unsupported xhigh to high. Scope: Stage 1 requires lifecycle and completion telemetry to report the child's actual effective level, including when no override is supplied.
Finding 2 — Registry-less integrations reject non-off thinking deferred Factual: Without a registry, string resolution succeeds but no concrete Model object exists, so every non-off override is rejected. Scope: Production AgentSession always supplies the registry; lower-level registry-less embedding behavior is optional follow-up rather than required for safe Stage 1 delivery.
Finding 3 — Four levels lack child-launch argument coverage genuine Factual: Schema checks cover all literals, but launch assertions omit off, minimal, medium, and supported xhigh. Scope: Approved criteria require every valid explicit level to produce exactly one child argument pair; explicit off must remain distinct from omission.
Finding 4 — Child-effective metadata lacks end-to-end completion coverage genuine Factual: The differing requested/reported-level test stops at SubagentResult; parent tests inject fabricated results, and chain tests request the values they emit. Scope: Child-effective metadata through parent delivery and background_agent_end.steps is explicitly required new observability behavior.
Finding 5 — Skill tests weakly protect malformed-history refusal genuine Factual: Contract tests assert broad evidence/cold-start phrases but not the JSONL parse gate, loud malformed/unreadable refusal, affected-file reporting, or prohibition against treating broken history as cold-start. Scope: These mandatory skill behaviors need contract protection because Stage 1 intentionally implements the workflow only in SKILL.md.
Finding 6 — Thinking schema literals are duplicated nitpick Factual: The six literals appear in both the task-level and top-level schemas. Scope: They currently agree; consolidation is maintainability-only.
Finding 7 — Result metadata formatting is duplicated nitpick Factual: Two call sites independently construct matching model/thinking strings. Scope: Current output is consistent, so a helper would be optional refactoring.
Finding 8 — Thinking precedence helper wraps only ?? nitpick Factual: The helper returns only taskThinking ?? topLevelThinking. Scope: It names the precedence policy; inlining changes no requirement or behavior.
Finding 9 — Model-less guard overlaps validator behavior nitpick Factual: The guard overlaps undefined-model rejection but provides richer remediation and permits model-less explicit off. Scope: Collapsing or documenting it further is a structural preference, not a correctness requirement.

Action Plan

  1. Resolve capability-aware effective thinking before emitting agent_start, including unsupported default or inherited xhigh, and test both loop entry points.
  2. Add integrated background and heterogeneous-chain tests proving child-reported canonical model/thinking values survive parent delivery and background_agent_end.
  3. Add spawn-path assertions for explicit off, minimal, medium, and supported xhigh, each producing exactly one argument pair.
  4. Strengthen the routing-skill contract test for malformed/unreadable history refusal, affected-file reporting, and strict cold-start semantics.

Optional Follow-up

Clarify or formally require modelRegistry for registry-less subagent integrations that request non-off thinking.


Assessment by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Progress Update

Fixed assessed review findings 1, 3, 4, and 5:

  • Effective thinking metadata now reports off for non-reasoning models and high when unsupported xhigh is capability-clamped.
  • Added regression coverage for both normal and continuation agent-loop entry points.
  • Added child-launch coverage for all six explicit thinking levels, including the distinction between explicit off and omission.
  • Added end-to-end single and heterogeneous-chain coverage from child lifecycle events through parent delivery and background_agent_end metadata.
  • Strengthened the routing-guide contract tests for unreadable/malformed JSONL refusal and strict cold-start semantics.
  • Updated model-availability probe expectations to reflect the corrected effective-level clamp.

Verification:

  • Commit hook passed 5,139 tests with 709 live tests skipped.
  • Full npm test passed.
  • npm run build passed.
  • npm run verify-workspace-links passed.
  • Biome and git diff --check passed.

Commit: d9b2da6


Progress tracked by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Manual QA Finding — Routing Guide Scope Confusion

A live manual invocation of /skill:model-routing-guide without arguments failed the intended happy path.

The model correctly read enabledModels from ~/.dreb/agent/settings.json and found a populated explicit model list, but then continued looking for a separate source of “scoped models” instead of treating that list as the authoritative scope and proceeding to model resolution.

Likely cause

The skill description and instructions use “explicitly scoped models,” “actual scoped provider/model combinations,” and “current session scope” in ways that suggest another runtime/session scope must be discovered. In the skill-only Stage 1 design, however, the workflow cannot inspect a session's runtime --models value or later in-session scope changes.

For a no-argument invocation, a non-empty effective enabledModels array is already the complete authoritative scope. There is no additional scoped-model source to search for.

The procedure also says to use the installed CLI/model listing without naming the concrete dreb --list-models command, leaving an unnecessary inference step in an already ambiguous section.

Required correction

  • Rewrite the skill description and Step 1 to state that scope has exactly two supported sources:
    1. Skill arguments, when present; otherwise
    2. The effective non-empty enabledModels array.
  • State explicitly: once either source is found, do not search for any other session-scoped model state.
  • State honestly that runtime --models scope is not discoverable by this skill-only workflow; users who want that exact scope must pass the same patterns as skill arguments.
  • Name dreb --list-models as the command used to obtain available canonical candidates.
  • Update documentation and contract tests to protect these semantics rather than only checking generic “scope” terminology.

Manual acceptance checks

  • Bare invocation with populated enabledModels immediately treats that array as authoritative and proceeds to candidate resolution.
  • Explicit skill arguments replace enabledModels as the authoritative scope.
  • Missing arguments plus missing/empty enabledModels fails loudly.
  • The workflow never searches for another runtime/session scope after selecting either supported source.

Manual QA tracked by mach6

@m-aebrer

Copy link
Copy Markdown
Collaborator Author

Progress Update

Fixed the routing-guide scope confusion found during manual QA:

  • Rewrote the skill description and first workflow step around exactly two authoritative sources: non-empty skill arguments, otherwise the effective non-empty enabledModels array.
  • Explicitly tells the model to stop looking for scope after selecting either source and never search for a separate runtime/session “scoped models” value.
  • Documents that a skill-only workflow cannot discover runtime --models or later in-session scope changes; users must repeat those patterns as skill arguments.
  • Names dreb --list-models as the available-model command and documents project-over-global enabledModels replacement semantics.
  • Updated the root README, coding-agent README, skills documentation, and agent-model documentation to use the same factual contract.
  • Added dedicated contract coverage for source precedence, authoritative enabledModels, runtime-scope limitations, the concrete CLI command, and prohibited misleading wording.

Verification:

  • Focused routing-skill tests passed: 40 tests.
  • Complete non-live workspace suite passed.
  • Commit hook passed 5,140 tests with 709 live tests skipped.
  • npm run build passed.
  • npm run verify-workspace-links passed.
  • Biome and git diff --check passed.
  • An unrestricted live-suite attempt reached the external OpenAI Codex usage limit; no non-live test failed.

Commit: fefef80


Progress tracked by mach6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant