Skip to content

perf: skip manifest normalization in status summary#87831

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
ACSKamloops:perf/status-summary-manifest-skip-20260528-refresh
Jun 28, 2026
Merged

perf: skip manifest normalization in status summary#87831
vincentkoc merged 1 commit into
openclaw:mainfrom
ACSKamloops:perf/status-summary-manifest-skip-20260528-refresh

Conversation

@ACSKamloops

@ACSKamloops ACSKamloops commented May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

What problem does this PR solve?

  • status/status --json builds model labels for configured and persisted
    session rows in a shallow read-only path.
  • That path already avoids plugin/runtime model normalization, but it still let
    manifest model-id normalization run repeatedly while building status rows.

Why does this matter now?

  • Status commands are used as operational readbacks, including during incidents
    and package validation.
  • Current main is much better than the original local slowdown, but repeated
    status row parsing still pays avoidable manifest-normalization cost.

What is the intended outcome?

  • Keep status summary model parsing on the cheap literal/configured path.
  • Preserve existing model resolver behavior for callers that do not opt out.
  • Add regression coverage that status summary does not call manifest or
    plugin/runtime normalization for configured or providerless persisted session
    model values.

What is intentionally out of scope?

  • No full status-summary row-construction refactor.
  • No duplicate-row caching change.
  • No provider/model catalog behavior change outside status summary.
  • No CHANGELOG.md edit.

What does success look like?

  • statusSummaryRuntime passes allowManifestNormalization:false in its shallow
    configured and persisted session model paths.
  • Persisted resolver helpers thread the optional flag without changing defaults.
  • Focused tests, formatting, diff check, package build, package integrity, and
    temp-prefix install pass on the rebased branch.

What should reviewers focus on?

  • Whether the shallow status path is the right caller to skip manifest
    normalization.
  • Whether the persisted resolver option remains safely defaulted for all other
    callers.
  • Whether the real-behavior proof is sufficient for this bounded performance
    improvement.

Linked context

Which issue does this close?

Closes #

Which issues, PRs, or discussions are related?

Related #80611
Related #80614

Was this requested by a maintainer or owner?

No maintainer request. This is a narrow follow-up from local status-summary
performance investigation and current-main package proof.

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: shallow status-summary model-label rendering can
    repeatedly invoke manifest model-id normalization even though status is a
    read-only display path that already opts out of plugin/runtime normalization.
  • Real environment tested: local Linux OpenClaw source checkout rebased onto
    origin/main at 5d8cf28578a1c5b4025a7d8da1b73d1dffc047c3, head
    50bc288c4dabaac33b8675ec54818eed7e9520fc, Node v22.22.2, packaged
    OpenClaw 2026.5.28 (50bc288) installed into a temporary npm prefix.
  • Exact steps or command run after this patch: ran focused Vitest targets via
    node scripts/test-projects.mjs, checked formatting with oxfmt, ran
    git diff --check, rebuilt with node --import tsx scripts/openclaw-prepack.ts,
    ran npm pack, checked the tarball with
    node scripts/check-openclaw-package-tarball.mjs, installed the tarball in a
    temp prefix, read back openclaw --version, and checked built markers.
  • Evidence after fix:
    Copied terminal output from the rebased source and package proof:
Current base main: 5d8cf28578a1c5b4025a7d8da1b73d1dffc047c3
Current source head: 50bc288c4dabaac33b8675ec54818eed7e9520fc

Focused post-rebase tests:
- status.summary.runtime.test.ts: 11/11 passed
- test-live-shard.test.ts: 9/9 passed
- status.summary.runtime.normalization.test.ts: 2/2 passed
- model-selection.test.ts: 106/106 passed

Post-rebase checks:
- oxfmt check passed on the three touched files
- git diff --check passed

Package:
- temp-prefix version: OpenClaw 2026.5.28 (50bc288)
- tarball SHA256: 1f5f47945401a97c73320c6b17cae109235c91ace9159f242ad34cc96f0813e9
- tarball integrity: passed
- status runtime markers: three allowManifestNormalization:false paths
- persisted resolver threading markers: present
- generated .d.ts allowManifestNormalization?: boolean: present

Previously generated source benchmark from the same narrow patch lane:
- synthetic workload: 11 agents x 20 sessions each
- baseline median: 54.5ms
- patched median: 18.4ms
- median improvement: 66.2%
- baseline cold: 447ms
- patched cold: 397.2ms
- parseModelRef default manifest-normalization median: 45.5ms / 500 parses
- parseModelRef allowManifestNormalization:false median: 0.2ms / 500 parses
  • Observed result after fix: status summary configured-model and persisted
    session model parsing stay on the shallow path, the package installs in a
    temp prefix, and the direct hot operation avoids manifest-normalization cost.
  • What was not tested: no live Gateway install of this PR package, no full CI,
    no Kova workflow dispatch, no live-provider run, and no broad status-summary
    refactor.
  • Proof limitations or environment constraints: the synthetic source benchmark
    uses local generated session stores so it is reproducible and private-data
    free. It was generated earlier in this current-main prep lane; after PR review
    and upstream drift, the branch was rebased onto 5d8cf285, and focused
    tests/package proof were rerun on the rebased head.
  • Before evidence (optional but encouraged): before patch on the refreshed
    benchmark base, the same synthetic status workload had 54.5ms warm median
    and the direct parse microbench had 45.5ms median for 500 calls with default
    manifest normalization.

Tests and validation

Which commands did you run?

  • node scripts/test-projects.mjs src/commands/status.summary.runtime.normalization.test.ts src/commands/status.summary.runtime.test.ts src/agents/model-selection.test.ts test/scripts/test-live-shard.test.ts
  • ./node_modules/.bin/oxfmt --check --threads=1 src/agents/model-selection.ts src/commands/status.summary.runtime.ts src/commands/status.summary.runtime.normalization.test.ts
  • git diff --check origin/main...HEAD
  • PATH=/usr/lib/node_modules/corepack/shims:$PATH node --import tsx scripts/openclaw-prepack.ts
  • npm pack --ignore-scripts --pack-destination <artifact-dir>
  • node scripts/check-openclaw-package-tarball.mjs <tarball>
  • npm install --global --prefix <temp-prefix> <tarball>
  • <temp-prefix>/bin/openclaw --version
  • built marker readback with rg

What regression coverage was added or updated?

  • Added src/commands/status.summary.runtime.normalization.test.ts.
  • The new test mocks both manifest normalization and plugin/runtime
    normalization and verifies status summary does not call either path for:
    • configured direct model refs
    • configured aliases
    • providerless persisted runtime models
    • providerless persisted model overrides

What failed before this fix, if known?

  • Current main still calls manifest normalization from the status summary
    configured and persisted session model parse paths.
  • In direct source benchmarking, default manifest normalization took 45.5ms
    median per 500 parseModelRef calls, while the status opt-out path took
    0.2ms.

If no test was added, why not?

  • A targeted regression test was added.

Risk checklist

Did user-visible behavior change? (Yes/No)

Yes, narrowly: status summary may display providerless persisted session model
values literally, for example anthropic/opus-4.6, instead of applying plugin
manifest aliases in this shallow status path.

Did config, environment, or migration behavior change? (Yes/No)

No.

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No)

No.

What is the highest-risk area?

The intentional status-display tradeoff for providerless persisted model aliases.

How is that risk mitigated?

  • The option defaults to existing behavior for all other callers.
  • Only status summary passes allowManifestNormalization:false into the
    persisted model resolver chain.
  • Focused tests cover configured and persisted status paths.
  • Built package marker readback confirms the expected status path and type
    surface are present.

Current review state

What is the next action?

Maintainer review after CI reruns on the rebased head. ClawSweeper says no
contributor repair is needed; the maintainer decision is whether to accept the
shallow-status display tradeoff.

What is still waiting on author, maintainer, CI, or external proof?

  • CI: rerun on the rebased head after the branch update.
  • Maintainers: judge whether shallow status should skip manifest normalization
    and display providerless persisted model values literally in this read-only
    status path.

Which bot or reviewer comments were addressed?

  • ClawSweeper reviewed the previous head, marked proof sufficient, rated the PR
    platinum hermit, and said no ClawSweeper repair is needed before maintainer
    review.
  • ClawSweeper's only rank-up move is maintainer acceptance of the literal-label
    status tradeoff before merge.
  • The old build-artifacts failure was traced to upstream drift in
    test/scripts/test-live-shard.test.ts. Current origin/main includes the
    upstream fix (f09b69a7 test: drop removed gateway live shard fixture), and
    the rebased branch now passes that focused test locally.

@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations agents Agent runtime and tooling size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 29, 2026
@clawsweeper

clawsweeper Bot commented May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 21, 2026, 1:45 AM ET / 05:45 UTC.

Summary
The PR adds an optional manifest-normalization opt-out to persisted model resolvers, disables manifest normalization in status-summary model parsing, and adds focused Vitest coverage for configured and providerless persisted status labels.

PR surface: Source +11, Tests +100. Total +111 across 3 files.

Reproducibility: yes. Source inspection shows current status summary disables plugin normalization without disabling manifest normalization, and the PR body provides after-fix terminal proof plus before/after benchmark output for that path.

Review metrics: 1 noteworthy metric.

  • Resolver option surface: 1 optional flag added, threaded through 3 resolver helpers; 1 status caller opts out. This is the new compatibility-sensitive switch that lets status change display parsing while preserving defaults for other resolver callers.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Have a maintainer explicitly accept or reject the literal providerless status-label behavior before merge.

Risk before merge

  • [P1] Status output may intentionally display providerless persisted model aliases literally, such as anthropic/opus-4.6, instead of applying plugin manifest aliases; existing users or scripts that read status labels could notice this after upgrade.
  • [P1] The broader metadata-reuse work already reduced the original incident shape, so maintainers should decide whether this residual status-specific optimization is still worth the display-semantic tradeoff.

Maintainer options:

  1. Accept literal status labels for the shallow path
    Maintainers can merge the narrow optimization if providerless persisted model values displaying literally is acceptable for read-only status summaries.
  2. Keep normalized labels and optimize elsewhere
    If status output must preserve manifest-normalized aliases, require a revised patch that avoids repeated metadata cost without changing displayed model labels.
  3. Pause until current-main timing demands it
    If the merged metadata-reuse work already makes status fast enough, maintainers can pause or close this follow-up until a current-main timing report proves the remaining cost matters.

Next step before merge

  • No automated repair is indicated; the remaining action is maintainer acceptance or rejection of the status-label compatibility tradeoff.

Security
Cleared: The diff only changes TypeScript resolver options and focused Vitest coverage, with no dependency, workflow, secret, network, install, or supply-chain surface changed.

Review details

Best possible solution:

Merge only after maintainers accept the shallow-status label tradeoff; otherwise preserve manifest-normalized display semantics and pursue a different status performance path.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows current status summary disables plugin normalization without disabling manifest normalization, and the PR body provides after-fix terminal proof plus before/after benchmark output for that path.

Is this the best way to solve the issue?

Yes for the code shape: the status summary is already a shallow read-only caller, and threading an opt-out flag keeps defaults unchanged for other persisted resolver callers. The unresolved question is whether maintainers accept the intentional literal-label status behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4b2298e8cbcb.

Label changes

Label justifications:

  • P2: This is a bounded status performance and display-semantics change with limited blast radius but real maintainer review value.
  • merge-risk: 🚨 compatibility: The PR intentionally changes status label rendering for providerless persisted aliases, which existing users or scripts may observe after upgrade.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes copied after-fix terminal output for focused tests, formatting, package build/pack/install, version readback, marker checks, and before/after benchmark numbers for the changed status path.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied after-fix terminal output for focused tests, formatting, package build/pack/install, version readback, marker checks, and before/after benchmark numbers for the changed status path.
Evidence reviewed

PR surface:

Source +11, Tests +100. Total +111 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 2 11 0 +11
Tests 1 100 0 +100
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 111 0 +111

What I checked:

  • Current status configured-model path still permits manifest normalization: Current main calls parseModelRef from resolveStatusModelRefFromRaw with allowPluginNormalization:false but no allowManifestNormalization:false, so manifest model-id normalization can still run in status summary. (src/commands/status.summary.runtime.ts:41, 4b2298e8cbcb)
  • Current persisted session status path still lacks manifest opt-out: Current main resolves persisted selected session models with allowPluginNormalization:false only, matching the PR's targeted gap. (src/commands/status.summary.runtime.ts:116, 4b2298e8cbcb)
  • Manifest normalization is controlled separately: normalizeStaticProviderModelId skips manifest policies only when allowManifestNormalization is explicitly false; otherwise it calls normalizeProviderModelIdWithManifest before built-in normalization. (src/agents/model-ref-shared.ts:67, 4b2298e8cbcb)
  • Status summary is a shallow read-only caller: getStatusSummary uses statusSummaryRuntime to resolve configured and session model labels while explicitly keeping context lookup async loading disabled, so the changed caller is the status display path rather than provider execution. (src/commands/status.summary.ts:324, 4b2298e8cbcb)
  • Sibling resolver callers keep existing defaults: Gateway/session, subagent, live-model-switch, and auto-reply callers currently pass no manifest-normalization option, so adding the optional flag preserves existing behavior unless a caller opts out. (src/gateway/session-utils.ts:1670, 4b2298e8cbcb)
  • PR diff applies the opt-out only to status and threads the option: The patch adds allowManifestNormalization?: boolean to the persisted resolver parameter shapes, forwards it into parseModelRef, and passes false from the status summary runtime. (src/agents/model-selection.ts:97, 50bc288c4dab)

Likely related people:

  • vincentkoc: Current local blame maps the status-summary runtime and persisted resolver lines to a recent Vincent Koc commit, and GitHub history shows earlier status runtime/session display work in the same area. (role: recent status-runtime contributor; confidence: high; commits: b43eedbb1818, 46c99cff0b2f, b338a68e57c1; files: src/commands/status.summary.runtime.ts, src/agents/model-ref-shared.ts)
  • steipete: GitHub history shows Peter Steinberger introduced or shaped the status plugin-normalization fast path and persisted session model resolver helpers that this PR extends. (role: model-selection and status fast-path contributor; confidence: high; commits: e61eba11e671, 1fb44f0aadd4, 32ebaa37574e; files: src/commands/status.summary.runtime.ts, src/agents/model-selection.ts, src/agents/model-ref-shared.ts)
  • shakkernerd: The merged metadata-reuse PR that closed the broader linked status metadata regression was authored by Shakker and overlaps the manifest metadata performance root-cause family. (role: related metadata-reuse feature owner; confidence: medium; commits: f5904392e9a2, 20704ffab728, c90e42aaa766; files: src/agents/model-selection.ts, src/agents/model-selection-shared.ts, src/plugins/runtime/load-context.ts)
  • rolandrscheel: GitHub history shows earlier workspace metadata reuse for session model refs and manifest model-id normalization paths adjacent to this PR's performance concern. (role: adjacent metadata normalization contributor; confidence: medium; commits: aeac10f5cea7; files: src/agents/model-ref-shared.ts, src/agents/model-selection-normalize.ts, src/plugins/manifest-model-id-normalization.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels May 29, 2026
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. proof: sufficient ClawSweeper judged the real behavior proof convincing. labels May 29, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 29, 2026
@ACSKamloops
ACSKamloops force-pushed the perf/status-summary-manifest-skip-20260528-refresh branch from 239f43b to 50bc288 Compare May 29, 2026 01:21
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 29, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 29, 2026
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels May 29, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented Jun 15, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 15, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 15, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 15, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 16, 2026
@clawsweeper clawsweeper Bot added status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jun 19, 2026
@vincentkoc
vincentkoc merged commit 87aae9f into openclaw:main Jun 28, 2026
216 of 239 checks passed
@github-actions github-actions Bot mentioned this pull request Jun 28, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 29, 2026
@ACSKamloops
perf: skip manifest normalization in status summary (openclaw#87831)
53c01f0
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
@ACSKamloops @QiuYuang
perf: skip manifest normalization in status summary (openclaw#87831)
fdefbe9
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026
@ACSKamloops chenyangjun-xy
perf: skip manifest normalization in status summary (openclaw#87831)
b8f6584
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026
@ACSKamloops @chenyangjun-xy
perf: skip manifest normalization in status summary (openclaw#87831)
ee13a3f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling commands Command implementations merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ACSKamloops @vincentkoc @RomneyDa