Skip to content

refactor: streamline skills JSON output - #140581

Merged
steipete merged 1 commit into
mainfrom
codex/perf54-skills-json-serialization
Sep 7, 2026
Merged

refactor: streamline skills JSON output#140581
steipete merged 1 commit into
mainfrom
codex/perf54-skills-json-serialization

Conversation

@steipete

@steipete steipete commented Sep 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
Additional instructions

MUST: Keep Allow edits from maintainers enabled for this PR so maintainers
can help update the branch when needed.

What Problem This Solves

Large openclaw skills list --json inventories spend avoidable time formatting their reports. This reduces that work while preserving the machine-readable output contract and terminal-control sanitization.

Why This Change Was Made

Apply the existing string sanitizer during JSON serialization instead of recursively copying the report first. Also prepare the human lookup-error name only when returning that error. The cleanup removes 11 production lines without adding dependencies, configuration, or API fields.

The contract is the plain status reports produced by Gateway/local discovery, not arbitrary class instances or custom toJSON methods.

User Impact

The fixed cohort's clean 1,000-skill list JSON medians improved 14.74% / 14.79%, saving about 0.404 / 0.428 ms per formatting call. Control-bearing 1,000-skill lists improved 13.04% / 12.63%. All returned output bytes were preserved.

The benefit is localized, not uniform: clean 1,000-skill info JSON regressed 55.22% / 69.55% (16.16 / 21.77 µs), some check rows regressed roughly 1–8%, and human-output controls were mixed and frequently adverse. Kernel and sampled tree peak RSS increased about 0.8–1.1%. No overall CLI-startup, Gateway, or memory improvement is claimed.

Evidence

  • 37 tests across the two existing skills CLI test files passed on the final source; native wall time was 9.13 seconds. All 28 normal checks passed on that source in 304.76 seconds, without retry or source changes during validation.
  • Fresh canonical P2 review passed, scoped-clean with no actionable findings (39.53 seconds). Installation/readiness and the unchanged path-only plan are explicitly retained from the preceding source revision.
  • Fixed 53-row B0/C0/C1/B1 measurements preserve 4,028 complete reconstructable returns: 3,116 independent JSON-value and pretty-byte checks, plus 912 human-text byte comparisons. Every call also checked unchanged input/options. An independent data audit reconstructed every return and reproduced all statistics.
  • 39/106 medians, 39/106 means, and 45/106 maxima are adverse. JSON rows contribute 21/82 adverse medians; unchanged human-text controls contribute 18/24. All rows are included below. Whole-child wall improved 8.89% / 0.80%, but those clocks include loading, fixture/oracle work, and capture I/O.

First observations and warmups were untimed correctness checks. Each reported sample is the mean of two separately clocked formatter calls; maxima are maximum batch means, not latency percentiles. Discovery and provider/Gateway execution are outside the measured formatter intervals.

The first capture attempt hit the unchanged 64 MiB journal cap after 948 verified returns and produced no performance verdict. R2 retained full strings in a lossless dictionary with one exact reference per return, including new differing outputs before assertions, then ran a fresh complete cohort. No cap or call count was increased/reduced, no partial R1 timing was pooled, and all failed-attempt evidence remains preserved. Capture changes happened outside the clock but can affect process state, so these are descriptive workload results rather than a universal speedup.

All 53 rows: forward and reverse median / mean / maximum changes

Percent change is candidate relative to baseline; negative is faster. Forward is B0→C0 and reverse is B1→C1. “controls” fixtures contain ANSI/C0/C1-bearing strings.

Row Forward median / mean / max % Reverse median / mean / max %
clean-0-list-json +7.39 / -10.15 / -37.53 -19.65 / -24.09 / -38.67
clean-0-list-eligible-json -2.56 / -0.78 / +48.57 +0.00 / -8.69 / +13.93
clean-0-check-json -31.45 / -18.01 / +59.59 -35.64 / -92.25 / -98.50
clean-0-info-json -2.77 / +13.63 / +87.96 -7.01 / -4.62 / -8.41
clean-1-list-json -15.40 / -17.00 / -14.29 -18.65 / -21.94 / -16.34
clean-1-list-eligible-json -19.19 / -29.61 / -47.28 -26.92 / -26.13 / -34.53
clean-1-check-json -10.87 / -18.00 / -26.79 -19.95 / -16.65 / -14.91
clean-1-info-json -15.03 / -8.80 / +26.62 -24.56 / -18.70 / +6.87
clean-20-list-json -17.62 / -19.49 / -19.94 -19.47 / -24.01 / -37.44
clean-20-list-eligible-json -10.90 / -20.77 / -31.01 -20.21 / -21.54 / -21.76
clean-20-check-json +2.81 / -2.50 / -12.47 -2.79 / -6.39 / -19.29
clean-20-info-json -23.97 / -27.53 / -48.07 -22.29 / -20.45 / -19.91
clean-200-list-json -15.21 / -25.32 / -58.10 -13.98 / -27.30 / -62.83
clean-200-list-eligible-json -10.71 / -25.46 / -62.08 -17.77 / -17.23 / -20.74
clean-200-check-json +8.44 / +5.62 / +3.27 +2.40 / +21.55 / +136.52
clean-200-info-json -13.56 / +2.52 / -8.51 +32.88 / +24.63 / -12.64
clean-1000-list-json -14.74 / -15.49 / -23.29 -14.79 / -15.80 / -23.95
clean-1000-list-eligible-json -10.09 / -11.96 / -20.69 -15.04 / -13.98 / -20.25
clean-1000-check-json +7.63 / +6.57 / +5.39 +1.22 / -0.94 / +3.34
clean-1000-info-json +55.22 / +29.70 / +12.22 +69.55 / +70.93 / +38.10
controls-0-list-json +1.19 / +4.34 / +8.20 -4.52 / -10.49 / -38.32
controls-0-list-eligible-json +3.42 / +1.96 / -1.00 +6.85 / +27.78 / +110.79
controls-0-check-json +0.22 / +17.67 / +50.00 -9.24 / -10.77 / +4.35
controls-0-info-json -3.97 / +1.01 / +17.29 -13.73 / -9.40 / -17.92
controls-1-list-json +0.57 / +26.68 / +195.09 +0.15 / -2.73 / -18.40
controls-1-list-eligible-json -1.42 / +3.04 / +31.50 -4.43 / -2.77 / +8.58
controls-1-check-json -7.42 / +19.32 / +179.87 -2.10 / -2.41 / -2.54
controls-1-info-json -7.52 / -5.33 / +12.88 -8.34 / -8.14 / -9.39
controls-20-list-json -5.43 / -8.41 / -18.10 -6.76 / -9.93 / -17.77
controls-20-list-eligible-json -5.80 / -27.91 / -60.67 -0.80 / -27.13 / -69.93
controls-20-check-json +7.34 / +11.73 / +12.10 +2.64 / +5.35 / +16.31
controls-20-info-json +15.75 / +22.68 / +43.54 -2.60 / -2.87 / -5.27
controls-200-list-json -2.67 / -7.62 / -25.52 -10.27 / -12.17 / -26.21
controls-200-list-eligible-json -13.70 / -15.27 / -26.65 -11.18 / -14.05 / -34.03
controls-200-check-json +8.28 / +4.17 / -3.99 +4.41 / +2.19 / +0.33
controls-200-info-json -23.53 / -8.43 / +44.62 -10.77 / -12.08 / +1.94
controls-1000-list-json -13.04 / -12.61 / -10.81 -12.63 / -13.47 / -15.53
controls-1000-list-eligible-json -6.09 / -7.65 / -14.55 -13.67 / -13.08 / -14.95
controls-1000-check-json -1.24 / -0.63 / -1.53 -0.62 / -2.12 / -9.24
controls-1000-info-json +44.83 / +9.42 / +4.37 -34.09 / -22.42 / -25.94
clean-1-list-text +6.81 / +1.83 / -2.80 -9.45 / -5.67 / +1.72
clean-1-check-text +6.70 / +3.92 / +7.04 +1.15 / -0.45 / -6.50
clean-1-info-text -3.03 / -21.34 / -60.34 -2.58 / -4.38 / -11.25
clean-200-list-text +3.61 / +2.82 / +5.82 +8.00 / +15.54 / +55.98
clean-200-check-text +15.54 / +12.47 / +24.64 +23.61 / +32.01 / +91.38
clean-200-info-text +9.38 / +46.40 / +76.41 +29.92 / +135.40 / +212.83
controls-1-list-text +11.74 / +83.24 / +521.44 +7.53 / +159.88 / +819.90
controls-1-check-text -2.21 / -0.25 / +3.21 +22.13 / +163.43 / +761.18
controls-1-info-text -5.31 / -7.74 / -18.93 +41.67 / +37.74 / +56.77
controls-200-list-text +0.94 / +1.12 / +3.42 +0.24 / +0.99 / +3.19
controls-200-check-text +6.12 / +1.92 / -2.30 -1.91 / +1.01 / -0.35
controls-200-info-text +12.66 / -6.87 / -33.89 +0.62 / +24.02 / +104.19
missing-controls-info-json -13.20 / -37.81 / -63.13 -10.10 / -10.93 / -11.28

Exact-head CI and required gate passed.

@steipete
refactor: streamline skills JSON serialization
6db280a 
Use the existing string sanitizer during JSON serialization instead of recursively cloning plain skill reports first. Only prepare the human missing-skill name when that branch needs it. Preserve report values, ordering, and formatting while reducing intermediate traversal and code.
@clawsweeper

clawsweeper Bot commented Sep 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@openclaw-barnacle openclaw-barnacle Bot added cli CLI command changes size: XS maintainer Maintainer-authored PR labels Sep 7, 2026
@steipete
steipete marked this pull request as ready for review September 7, 2026 01:35
@clawsweeper clawsweeper Bot added P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. 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 Sep 7, 2026
@clawsweeper

clawsweeper Bot commented Sep 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 6, 2026, 9:54 PM ET / September 7, 2026, 01:54 UTC.

ClawSweeper review

What this changes

The PR sanitizes skills JSON during serialization instead of copying reports recursively, and prepares human-readable lookup errors only when needed.

Merge readiness

✅ Ready for maintainer review

Keep open: this is a bounded, useful optimization not present on inspected main, with no actionable correctness finding. The maintainer-authored, protected PR remains eligible for normal review.

Priority: P3
Reviewed head: 6db280aceb1e3751c8792c660d41d81fefe32b5f

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused simplification with an appropriate plain-data contract, existing behavioral coverage, and candid performance tradeoffs.
Proof confidence 🌊 off-meta tidepool Not applicable: The supplied MEMBER association exempts this maintainer-authored PR from the external-contributor proof gate. Its benchmark exercises the changed formatters and reports preserved outputs, but excludes discovery and Gateway execution; no authority-chain exception applies.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The supplied MEMBER association exempts this maintainer-authored PR from the external-contributor proof gate. Its benchmark exercises the changed formatters and reports preserved outputs, but excludes discovery and Gateway execution; no authority-chain exception applies.
Evidence reviewed 7 items Verified introduced change: The locally inspected merge-base-to-head diff matches the complete host introduction evidence: only the formatting implementation changes. The host-verified test merge also changes only this file relative to its pinned main parent.
Production input contract: Status discovery constructs plain records containing primitives, arrays, requirements, installation options, and status metadata. JSON serialization's replacer visits nested string values without needing a recursive copy; custom class serialization is not part of this producer's contract.
CLI boundary: The CLI obtains skills.status from the Gateway or builds a local report, then writes the formatter's result to stdout. The patch leaves that routing and output destination intact.
Findings None None.
Security None None.

How this fits together

The skills CLI receives status reports from the Gateway or local discovery. Its formatting layer turns those reports into terminal text or sanitized JSON for scripts.

flowchart LR
  A[Gateway status] --> C[Skills status report]
  B[Local discovery] --> C
  C --> D{Output format}
  D --> E[Terminal text]
  D --> F[JSON serialization and string sanitization]
  F --> G[Standard output for scripts]
Loading

Before merge

None.

Agent review details

Security

None.

PR surface

Source -11. Total -11 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 13 24 -11
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 13 24 -11

Review metrics

Metric Value Why it matters
Reported formatter tradeoff 1,000-skill clean list: 14.74–14.79% faster; info: 16.16–21.77 µs slower The supplied measurements support a localized list-formatting benefit without establishing an overall CLI or memory improvement.

Technical review

Best possible solution:

Use the serializer's string replacer while preserving the existing plain-report JSON fields, ordering, indentation, and control-character sanitization.

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

Not applicable to a correctness bug: this PR optimizes existing formatting, and the body supplies workload-specific before/after measurements.

Is this the best way to solve the issue?

Yes. A JSON replacer removes redundant traversal while applying the same sanitizer to the plain nested data produced by discovery.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against dd3faf5f4749.

Labels

Label justifications:

  • P3: This is a low-risk formatting optimization with unchanged intended output and no urgent user-facing defect.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The supplied MEMBER association exempts this maintainer-authored PR from the external-contributor proof gate. Its benchmark exercises the changed formatters and reports preserved outputs, but excludes discovery and Gateway execution; no authority-chain exception applies.

Evidence

What I checked:

  • Verified introduced change: The locally inspected merge-base-to-head diff matches the complete host introduction evidence: only the formatting implementation changes. The host-verified test merge also changes only this file relative to its pinned main parent. (src/cli/skills-cli.format.ts:79, 6db280aceb1e)
  • Production input contract: Status discovery constructs plain records containing primitives, arrays, requirements, installation options, and status metadata. JSON serialization's replacer visits nested string values without needing a recursive copy; custom class serialization is not part of this producer's contract. (src/skills/discovery/status.ts:52, 6db280aceb1e)
  • CLI boundary: The CLI obtains skills.status from the Gateway or builds a local report, then writes the formatter's result to stdout. The patch leaves that routing and output destination intact. (src/cli/skills-cli.ts:188, 6db280aceb1e)
  • Existing behavioral coverage: Existing tests assert parsed list/info/check output, ANSI and C1 removal, and sanitized missing-skill errors; discovery-backed formatting coverage also checks nested requirements. Tests were inspected, not executed during this read-only review. (src/cli/skills-cli.test.ts:565, 6db280aceb1e)
  • Current-main necessity and release check: Inspected main still uses recursive sanitization, so the proposed optimization remains distinct. Reading the latest release's formatter failed because its blob was unavailable and network resolution failed; no release inclusion is claimed. (src/cli/skills-cli.format.ts:79, dd3faf5f4749)
  • Area history and routing: Main history identifies Peter Steinberger's prior work on this formatter's verbose-list performance, requirements presentation, and JSON failure contract. Exact-line blame was unavailable, so this establishes a routing candidate without claiming introduction. (src/cli/skills-cli.format.ts, 44378c46bb04)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (1 earlier review cycle)
  • reviewed 2026-09-07T01:41:08.993Z sha 6db280a :: needs maintainer review before merge. :: none

@steipete
steipete merged commit 7ca372a into main Sep 7, 2026
176 checks passed
@steipete
steipete deleted the codex/perf54-skills-json-serialization branch September 7, 2026 02:03
@github-actions github-actions Bot mentioned this pull request Sep 7, 2026
Open
@steipete

steipete commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash; the accepted merge was confirmed after main advanced during final verification. No second merge request was sent.

@Haderach-Ram Haderach-Ram mentioned this pull request Sep 7, 2026
Open
vincentkoc added a commit that referenced this pull request Sep 7, 2026
@vincentkoc
Merge commit '8a540c252002878f3c3e9d8dfb837b83c8602a78' into temp/pr-…
9c19c1e 
…137098

* commit '8a540c252002878f3c3e9d8dfb837b83c8602a78': (2210 commits)
  perf(plugins): keep bundled catalog lookups lightweight (#140597)
  refactor: streamline skills JSON serialization (#140581)
  fix(feishu): expand quoted merged-forward messages (#136231)
  fix(talk): keep opaque realtime routes out of public config (#129144)
  fix(ci): avoid Linux packaging on pull requests (#140589)
  fix(usage): release completed request payloads (#140588)
  fix(tooling): omit Trae machine squash credit (#140562)
  fix(ui): stop catalog paging in hidden tabs (#140583)
  fix(portals): reach IPv6 loopback with IPv4-only localhost records (#140580)
  fix(transcripts): preserve fixed and legacy capture history (#140563)
  fix(ui): prevent Talk E2E audio capture stalls (#138803)
  fix(media): release completed document source buffers (#140561)
  fix(anthropic): preserve resumed CLI caches across Git changes (#140566)
  chore(ui): refresh control ui locales (#140586)
  fix(ui): keep usage exports bound to their session instance (#140573)
  refactor(exa): test response readers through tool execution (#140572)
  fix(macos): omit incompatible worker binaries from app bundles (#140557)
  fix: avoid retaining discarded tool output (#140570)
  refactor(gateway): share placement move matching (#140569)
  fix(gateway): preserve child turns during systemd drain (#140568)
  ...

# Conflicts:
#	.agents/skills/release-openclaw-maintainer/SKILL.md
#	docs/reference/RELEASING.md
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Sep 7, 2026
@steipete
refactor: streamline skills JSON serialization (openclaw#140581)
19b0337 
Use the existing string sanitizer during JSON serialization instead of recursively cloning plain skill reports first. Only prepare the human missing-skill name when that branch needs it. Preserve report values, ordering, and formatting while reducing intermediate traversal and code.
@steipete steipete mentioned this pull request Sep 7, 2026
Merged
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

cli CLI command changes maintainer Maintainer-authored PR P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS 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.

1 participant

@steipete