qwen-code/docs/design/rt-optimization
gwinthis 7bed56b9b6
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
feat(telemetry): foundation for skill-based RT optimization (P0+P1) (#4565)
* docs(design): add RT optimization design doc

Two-round review trail documenting the analysis path: original D1-D4
proposal, code-level verification in §6 that recanted the cost estimates,
and §7 ROI reordering after DashScope ephemeral cache implementation was
confirmed already in place — which collapsed D2's net benefit and led to
deferring D2 and D4 as won't-fix.

The doc is preserved as the canonical record of why the obvious-looking
directions (fast-model routing, prevalidate scheduling) turn out to be
dead ends, so future work doesn't relitigate the same conclusions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(design): add reduce-rounds-via-skill-design with spec-first gating

Companion design to rt-optimization-design.md. The core argument:
the real lever for reducing agent loop rounds is at the skill/tool
design layer, not the agent framework. Round 2 in §1.2's baseline
exists because Round 1's skill didn't return a complete answer —
fixing that per-skill collapses 3 rounds into 2, an angle the
original framework-centric proposal completely missed.

Layout:
- §0 acceptance spec is the front-loaded gate: engineering specs
  lock at P-1, statistical thresholds lock at P1.5 (after baseline),
  per-skill specs are data-driven and live in PR descriptions
- §3-§4 three-layer plan: telemetry → per-skill rewrites → prompt
  guidance for concurrent tool calls; each layer is independently
  measurable and reversible
- §5.3 stop-loss lines split into result + process metrics to catch
  the "looks like progress, no actual ROI" failure mode early

The doc was reviewed by codex twice — once on initial draft (caught
qwen-logger dead-code path, batch_size state-passing cost, prompts.ts
line drift) and once after §0 was added (caught spec rigidity, missing
per-skill template, framework boundary case). Both rounds' findings
were either applied or explicitly recorded as not-adopted with reasons
inline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(telemetry): connect logSkillLaunch to QwenLogger

logSkillLaunch in loggers.ts went only through the OTLP path, while
QwenLogger.logSkillLaunchEvent in qwen-logger.ts had no callers anywhere
in the repo — leaving the skill_launch event invisible to any backend
that consumes from the qwen-logger pipeline rather than OTLP.

Mirror the logToolCall pattern at loggers.ts:230: forward the event to
QwenLogger before the OTLP path so the call still reaches QwenLogger when
the OTEL SDK is not initialized.

This is P0 of docs/design/rt-optimization/reduce-rounds-via-skill-design.md
§4.1.1b — a prerequisite for the prompt_id propagation in P1 so the
SkillLaunchEvent / ToolCallEvent join in §4.1.2 has data to query against.

Tests: 2 new cases under describe('logSkillLaunch') covering forwarding
to QwenLogger plus the OTLP-uninitialized branch; loggers.test.ts now
47/47 pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(telemetry): thread prompt_id through SkillLaunchEvent

To join skill_launch events with the subsequent tool_call events they
trigger, SkillLaunchEvent now carries the prompt_id of the user turn
that fired the skill. The scheduler already holds the request and its
prompt_id; the missing piece was getting that id into the invocation
that does the actual logSkillLaunch call.

Wiring:
- SkillLaunchEvent constructor adds a required prompt_id parameter so
  the field can never be silently undefined in a backend join.
- SkillToolInvocation exposes setPromptId(id) and stores the value;
  the four logSkillLaunch sites in execute() pass this.promptId through.
- CoreToolScheduler.buildInvocation grew an optional fourth promptId
  argument and duck-types setPromptId on the freshly-built invocation,
  mirroring the existing setCallId hook. The two callers (setArgs path
  at L1036 and the main schedule path at L1497) pass
  request.prompt_id / reqInfo.prompt_id.
- qwen-logger.logSkillLaunchEvent forwards prompt_id in the RUM event
  properties so the join works on the qwen-logger pipeline too.

The empty-string default on SkillToolInvocation.promptId is deliberate:
direct invocations (e.g. buildAndExecute in tests) that skip the
scheduler still log a valid event, and downstream queries can filter
prompt_id != '' to exclude non-scheduled launches from joins.

Implements P1 of docs/design/rt-optimization/reduce-rounds-via-skill-design.md
§4.1.1 — required prerequisite for the SkillFollowupRecord SQL in §4.1.2.

Tests: 2 new cases in skill.test.ts cover the setPromptId path and the
empty-default path; loggers.test.ts updated for the new 3-arg signature.
256 tests pass across loggers / skill / coreToolScheduler suites.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(scheduler): cover prompt_id propagation through buildInvocation

The duck-typed setPromptId hook added in the previous commit went
through unit tests on each side independently — SkillToolInvocation
tests verified that setting the field changes the logged event, and
the loggers tests verified the SkillLaunchEvent shape — but the
integration point in CoreToolScheduler.buildInvocation that wires the
two together was only exercised indirectly. Same is true of the older
setCallId hook it mirrors, which had no test at all.

Two cases here close that gap on the scheduler side:

- A purpose-built PromptIdAwareTool whose invocation records every
  setPromptId call; the test schedules a request with a known
  prompt_id and asserts the invocation captured it. This is the
  positive contract.

- The existing TestApprovalTool (no setPromptId) scheduled through
  the same path to confirm the duck-type guard does not throw when
  the method is absent. This is the backward-compatibility contract
  that lets every existing tool keep working unchanged.

The two cases together pin both branches of the typeof check in
buildInvocation, so future refactors of that hook cannot regress
silently. 165 tests in the suite still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(telemetry,scheduler,skill): close P0/P1 coverage gaps

Three blind spots remained after the initial P0+P1 work — each one
was a path that the production code change had already touched
mechanically but no test was pinning it against future regressions.

qwen-logger.ts logSkillLaunchEvent now has two cases asserting that
prompt_id reaches the RUM event properties, on both the success and
failure branch. Previously the loggers.test.ts spy stopped at "method
was called" and never inspected the payload qwen-logger built.

skill.ts had four logSkillLaunch sites, but only the happy path and
the empty-default path were tested. The commandExecutor-success
branch (L386), not-found branch (L399), and thrown-exception branch
(L482) now each have a test that sets promptId, drives execute()
through that specific path, and asserts the emitted event carries
both the right success flag and the right prompt_id. This catches
the failure mode where someone later edits one of those branches
and forgets the promptId argument — replace_all guaranteed today's
correctness but no test would catch a regression tomorrow.

CoreToolScheduler.buildInvocation now has two direct unit tests
that exercise the method through a type-assertion cast. Reaching
the L1036 setArgs path through the public API would require mocking
modifyWithEditor + the filesystem + an editor type, which would
dwarf the change under test. The direct call covers both L1036 and
L1497 simultaneously: when promptId is supplied the duck-typed
setPromptId is invoked; when it is omitted, the captured field
stays undefined and no throw happens.

298 tests pass across loggers / qwen-logger / skill / scheduler suites.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* review(PR #4565): address Copilot + github-actions feedback

Five spots flagged by the automated review on #4565. Three were real
and worth fixing; two were comment-quality touch-ups that traveled
along with the same patch.

- SkillLaunchEvent.prompt_id is now optional with a default of '',
  removing the breaking-change footprint on the exported telemetry API.
  All current internal callers still pass the value explicitly through
  the SkillToolInvocation.promptId field, so the §0.1 spec ("prompt_id
  串联") is still enforced in production paths — type-level enforcement
  just steps aside in favor of API stability, with §0.5 治理 covering
  the discipline at the process layer.

- The skill-design doc §4.1.1 used to claim "BaseToolInvocation 已有
  request.prompt_id" which is wrong: BaseToolInvocation only holds
  params, and the prompt_id flows through CoreToolScheduler's duck-typed
  setPromptId hook (mirroring setCallId). The doc now reflects the
  actual implementation and notes that the earlier text was the bug.

- CoreToolScheduler.buildInvocation gained a short JSDoc explaining
  why the two extra args (callId, promptId) are optional — they
  match the existing duck-type pattern that lets older tools and
  non-scheduler call sites work without implementing the setters.

- skill.test.ts adds a one-comment note next to the first setPromptId
  cast explaining that setPromptId is a scheduler-only hook, not part
  of the public ToolInvocation interface.

- SkillToolInvocation.promptId field comment shrank from 8 lines to 2
  with a pointer to the design doc so the inline noise drops without
  losing the empty-string semantics.

Pre-existing scope-creep findings (Chinese-only doc, mock-config
duplication in scheduler tests, redundant optional-chain comment,
prompt_id sanitization for an internally-generated UUID) are
deliberately not addressed here — see the reply on PR #4565 for
disposition per item.

298 tests still pass across loggers / qwen-logger / skill / scheduler.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-29 02:17:40 +08:00
..
reduce-rounds-via-skill-design.md feat(telemetry): foundation for skill-based RT optimization (P0+P1) (#4565) 2026-05-29 02:17:40 +08:00
rt-optimization-design.md feat(telemetry): foundation for skill-based RT optimization (P0+P1) (#4565) 2026-05-29 02:17:40 +08:00