Skip to content

refactor(agents): reuse owned tool projection arrays - #139424

Merged
steipete merged 1 commit into
mainfrom
codex/perf-sep05-projection-array-ownership
Sep 5, 2026
Merged

steipete merged 1 commit into
mainfrom
codex/perf-sep05-projection-array-ownership

Conversation

@steipete

@steipete steipete commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Simplifies tool preparation in the native runtime and shared harness bridge, where unnecessary intermediate arrays obscure the distinction between private projections and retained refresh views.

Why This Change Was Made

Reuse the schema projector's freshly owned arrays at three handoffs instead of immediately copying them. Source-entry snapshots, both schema passes, independent prompt baselines, and in-place refresh of the published tool array remain unchanged.

User Impact

No user-visible behavior change. This is a small ownership cleanup, not a demonstrated general latency or memory improvement. Production is −1 LOC; preservation coverage is +47 test LOC.

Evidence

  • 85 tests passed before and after across the bridge, bundle preparation, schema projection, and prompt-policy suites. The full 29-stage changed gate passed after a retained formatting failure was corrected.
  • Independent code review completed eight passes with no actionable P0–P2 findings. The source/consumer review judged the three copies redundant and the retained-refresh test useful.
  • A fixed B0/C0/C1/B1 comparison completed 58,268 operations and 896 batch means. Ordinary primary medians were mixed: direct bridge −3.53%/+0.72%, bundle preparation −1.05%/+1.03% (forward/reverse). Prepare-plus-refresh medians were −1.27%/−0.83%, with worse p95 batch means in both orders. All 1,906 comparisons and 942 adverse outcomes are retained; no threshold-pass claim is made.
  • The original apparatus failed its 4 MiB runtime allocation on normal SQLite WAL state. That failure is preserved; the corrected apparatus retained it within the same combined 64 MiB cap, with unchanged workload and native time/RSS/process limits. SQLite and logging behavior were not changed. Normal quarantine logs, including the logger's explicit 66-record queue-drop marker per numeric phase, are retained.
Focused validation commands
node scripts/run-vitest.mjs \
  src/agents/harness/tool-surface-bridge.test.ts \
  src/agents/embedded-agent-runner/run/attempt-bundle-tools.test.ts \
  src/agents/tool-schema-projection.test.ts \
  src/agents/embedded-agent-runner/run/attempt-prompt-tool-policy.test.ts

node scripts/check-changed.mjs -- \
  src/agents/harness/tool-surface-bridge.ts \
  src/agents/embedded-agent-runner/run/attempt-bundle-tools.ts \
  src/agents/embedded-agent-runner/run/attempt-bundle-tools.test.ts

The added test uses the real schema projector to preserve retained views and cron/inherited capability captures across two refresh generations. It passes on both variants and is not a claimed bug reproduction. No import or package boundary changed; no live-provider, Gateway-turn, or cold-start improvement is claimed.

@clawsweeper

clawsweeper Bot commented Sep 5, 2026 •

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 agents Agent runtime and tooling size: S maintainer Maintainer-authored PR labels Sep 5, 2026
@steipete
steipete marked this pull request as ready for review September 5, 2026 21:48
@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 5, 2026
@clawsweeper

clawsweeper Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 5, 2026, 5:53 PM ET / 21:53 UTC.

ClawSweeper review

What this changes

The PR reuses freshly allocated tool-filter results in agent preparation and adds coverage for retained tool lists across refreshes.

Merge readiness

✅ Ready for maintainer review

Keep open: this is a coherent ownership cleanup, and current main still contains the redundant copies. No introduced correctness or security defect was found; maintainer-authored work is also protected from automatic closure.

Priority: P3
Reviewed head: 81fe9f678cade219424764746a26c57c6d983fbf

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, source-supported cleanup with useful preservation coverage and appropriately limited performance claims.
Proof confidence 🌊 off-meta tidepool Not applicable: The maintainer-authored internal refactor is exempt from contributor live-proof requirements and introduces no material authority change; reported tests and measurements support the ownership cleanup without proving a general speedup.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The maintainer-authored internal refactor is exempt from contributor live-proof requirements and introduces no material authority change; reported tests and measurements support the ownership cleanup without proving a general speedup.
Evidence reviewed 8 items Introduced change boundary: The complete host-pinned merge-base-to-head patch removes three array copies and adds preservation coverage. The verified test merge contains only the same three affected files; unrelated endpoint drift is not attributed to this PR.
Projection owns its output: The schema inspector allocates compatibleTools locally on every invocation. Source entries are separately snapshotted before schema getters run, so returning the compatible array directly does not alias the caller's input array.
Refresh and prompt ownership remain intact: Bundle refresh still projects into a fresh result and splices it into the published array. The inspected catalog and attempt callers retain that view, while harness prompt policy explicitly copies its baseline. Capability captures derive separate names and records rather than retaining the projection array.
Findings None None.
Security None None.

How this fits together

Agent tool preparation combines core and plugin tools, filters incompatible schemas, and prepares the tools exposed to models. Retained arrays and independent prompt snapshots keep tool availability consistent during refreshes.

flowchart TD
  A[Core and plugin tools] --> B[Policy and normalization]
  B --> C[Schema filtering]
  C --> D[Owned tool array]
  D --> E[Tool catalog and model tools]
  D --> F[Independent prompt snapshot]
  G[Tool refresh] --> C
  C --> H[Update retained view]
Loading

Before merge

None.

Agent review details

Security

None.

PR surface

Source -1, Tests +47. Total +46 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 2 3 4 -1
Tests 1 48 1 +47
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 51 5 +46

Review metrics

None.

Technical review

Best possible solution:

Reuse the projector-owned arrays while preserving source snapshots, both schema passes, independent prompt baselines, and in-place refresh.

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

Not applicable: the PR claims behavior-preserving cleanup, and its new test explicitly checks preservation rather than reproducing a defect.

Is this the best way to solve the issue?

Yes: direct reuse follows the existing projector's allocation contract; another copying helper would retain the redundancy without improving isolation.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add P3: This is a bounded internal cleanup with no claimed user-visible behavior change or demonstrated general performance improvement.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The maintainer-authored internal refactor is exempt from contributor live-proof requirements and introduces no material authority change; reported tests and measurements support the ownership cleanup without proving a general speedup.

Label justifications:

  • P3: This is a bounded internal cleanup with no claimed user-visible behavior change or demonstrated general performance improvement.
  • 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 maintainer-authored internal refactor is exempt from contributor live-proof requirements and introduces no material authority change; reported tests and measurements support the ownership cleanup without proving a general speedup.

Evidence

What I checked:

  • Introduced change boundary: The complete host-pinned merge-base-to-head patch removes three array copies and adds preservation coverage. The verified test merge contains only the same three affected files; unrelated endpoint drift is not attributed to this PR. (81fe9f678cad)
  • Projection owns its output: The schema inspector allocates compatibleTools locally on every invocation. Source entries are separately snapshotted before schema getters run, so returning the compatible array directly does not alias the caller's input array. (src/agents/tool-schema-projection.ts:128, 81fe9f678cad)
  • Refresh and prompt ownership remain intact: Bundle refresh still projects into a fresh result and splices it into the published array. The inspected catalog and attempt callers retain that view, while harness prompt policy explicitly copies its baseline. Capability captures derive separate names and records rather than retaining the projection array. (src/agents/embedded-agent-runner/run/attempt-bundle-tools.ts:244, 81fe9f678cad)
  • Meaningful preservation coverage: The added test uses the real schema filter across two refresh generations, checks quarantine and restoration of a bundled tool, verifies retained views and capability captures, and checks that core input remains intact. It protects refactor invariants rather than claiming a preexisting bug reproduction. (src/agents/embedded-agent-runner/run/attempt-bundle-tools.test.ts:378, 81fe9f678cad)
  • Current-main necessity: Fetched main retains both bridge copies and the bundle-preparation copy. No merged replacement was established. Live GitHub lookup and the latest-release source read failed because hostname resolution was unavailable; no shipped-version claim is made. (src/agents/harness/tool-surface-bridge.ts:146, 9387e3fd735d)
  • Implementation boundary: The changed bridge imports OpenClaw's own Code Mode and schema-filter implementation. The schema oracle is in packages/ai/src/providers/tool-schema-json-projection.ts, which was inspected directly. Documentation distinguishes OpenClaw Code Mode from Codex Code Mode; this array-ownership change supplies no affirmative dependency on the Codex implementation. Public docs: docs/tools/code-mode.md. (docs/tools/code-mode.md:25, 81fe9f678cad)

Likely related people:

  • Peter Steinberger: 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.

@steipete
steipete merged commit 1818681 into main Sep 5, 2026
249 checks passed
@steipete
steipete deleted the codex/perf-sep05-projection-array-ownership branch September 5, 2026 22:30
@steipete

steipete commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

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

Labels

agents Agent runtime and tooling 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: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant