perf(agents): reuse manifest metadata during model resolution#86552
Conversation
|
Codex review: needs maintainer review before merge. Reviewed May 25, 2026, 7:25 PM ET / 23:25 UTC. Summary PR surface: Source +174, Tests +313. Total +487 across 13 files. Reproducibility: yes. The source path on current main still eagerly builds the alias index, and the PR plus linked issue provide redacted real-session timings; this review did not independently run the live Discord/WhatsApp path. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Rebase onto current main, keep the lazy manifest reuse only where provider/model normalization semantics remain covered, and land this as the narrow hot-path fix while the linked broader eager-alias issue remains tracked separately. Do we have a high-confidence way to reproduce the issue? Yes. The source path on current main still eagerly builds the alias index, and the PR plus linked issue provide redacted real-session timings; this review did not independently run the live Discord/WhatsApp path. Is this the best way to solve the issue? Yes, with maintainer review. Reusing a resolver-scoped manifest context matches the repository hot-path guidance, but the dirty branch and provider-routing compatibility surface make rebase and owner review the right next step. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against bef0ba8f5a7e. Label changesLabel justifications:
Evidence reviewedPR surface: Source +174, Tests +313. Total +487 across 13 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Gilded Review Wisp Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
|
Addressed ClawSweeper's P2 on empty alias maps. Added regression coverage asserting empty alias maps do not call Local verification after the fix:
|
|
@clawsweeper re-review Addressed the remaining P2 by filtering alias candidates before resolving manifest metadata. Added regressions for:
Local verification after the fix:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Addressed the CI regression after the narrowed lazy manifest lookup change. Latest fix:
Local verification after the fix:
CI follow-up:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
5a112e2 to
e5e0e0b
Compare
e5e0e0b to
5ff9501
Compare
5ff9501 to
1308bd7
Compare
Co-authored-by: Alyana <alyana@lumina.local>
1308bd7 to
59f3546
Compare
|
Rewrote this onto current Behavior addressed: avoid repeated model-manifest metadata loading in configured model resolution while preserving alias, auth-profile, OpenRouter compat, and plugin-owned normalization behavior.
|
Summary
manifestPluginsthrough alias index construction and nested model normalization instead of reloading manifest metadata repeatedlyopenrouter:automanifest normalization working while proving metadata is only loaded oncePerformance gained: resolve_default_model time went down from 3.80s to 0.15s
AI assistance
This PR was AI-assisted. Alyana/Codex helped identify the resolver hot path, write the patch, add the regression test, and run local validation.
Understanding
The resolver uses manifest plugin metadata to normalize provider/model refs, including aliases such as
openrouter:autoand configured shorthand aliases. The issue was not that normalization was wrong, but that this codepath repeatedly reloaded the same manifest metadata while resolving aliases. This patch resolves the manifest plugin list once, preserves the default-manifest fallback behavior, and threads the resolved metadata through the nested resolver calls.openrouter:autoand configured shorthand aliases.Real behavior proof
#alyana, using the same runtime path that triggered the profiling trace.openrouter:autoand configured shorthand aliases.Session / prompt context
The issue was found from local OpenClaw runtime profiling during real Discord request handling. I am not pasting raw private Discord/session logs into the PR body, but the sanitized trace above captures the measured before/after resolver timings from the real setup.
Validation
pnpm vitest run src/agents/model-selection-manifest-workspace.test.tspnpm vitest run src/agents/model-selection-resolve.test.ts src/agents/model-selection.test.ts src/gateway/session-utils.test.tspnpm build && pnpm checkpnpm testattempted; broad suite still has unrelated failures in ACP env, CLI update/option-collision, and registry-backed channel contract shards, with no resolver/model-selection failures observedcodex review --base origin/mainattempted locally, but could not complete because local Codex/OpenAI auth is missing (401 Unauthorized)