Skip to content

fix(mapper): tighten monorepo review slices - #160

Closed
coletebou wants to merge 8 commits into
openclaw:mainfrom
coletebou:agent/apex-monorepo-mapping
Closed

fix(mapper): tighten monorepo review slices#160
coletebou wants to merge 8 commits into
openclaw:mainfrom
coletebou:agent/apex-monorepo-mapping

Conversation

@coletebou

Copy link
Copy Markdown
Contributor

Summary

  • bound review prompt size and preserve validated multi-entrypoint metadata
  • improve monorepo detection, route grouping, and generic-versus-specialized file ownership
  • tighten semantic classification so client code is not treated as CLI code while camel-case CLI names remain recognized
  • add regression coverage for APEX-shaped workspaces, ownership, package-manager detection, and mounted routes

Why

Large monorepos such as APEX could produce overlapping review slices, lose accepted entrypoint metadata, and misclassify client modules as CLI functionality. The mapper now uses validated entrypoints and narrower ownership boundaries while retaining legitimate semantic signals.

Validation

  • corepack pnpm format:check
  • corepack pnpm typecheck
  • corepack pnpm lint
  • corepack pnpm exec vitest run --silent — 895 passed, 1 skipped
  • corepack pnpm build
  • whole-branch autoreview against origin/main: Codex gpt-5.6-sol high and Claude Fable 5 high both returned 0 findings

coletebou and others added 8 commits July 13, 2026 22:16
Refresh project metadata during mapping, prefer specialized ownership, and bound review prompts so large workspaces produce accurate, reviewable features.
Apply the final prompt-byte limit to the fully rendered review and rebase generic entrypoints after specialized ownership is removed.
Keep validated owned files usable when every provider-suggested entrypoint is rejected by repository path filtering.
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 17, 2026
@clawsweeper

clawsweeper Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 16, 2026, 11:20 PM ET / July 17, 2026, 03:20 UTC.

Summary
Tightens monorepo feature ownership and route grouping, preserves validated agent entrypoints, refreshes detected project metadata, improves semantic and package-manager detection, and adds a configurable review-prompt byte limit with regression tests.

Reproducibility: yes. at the source-fixture level: the added tests encode overlapping generic ownership, rejected entrypoints, package-manager precedence, semantic misclassification, mounted routes, and prompt overflow. No high-confidence real-workspace reproduction transcript is supplied.

Review metrics: 3 noteworthy metrics.

  • Diff surface: 19 files; 794 additions, 290 deletions. The bug fix spans configuration, state orchestration, detection, mapper ownership, routes, prompts, documentation, and tests rather than one isolated mapper helper.
  • Commit series: 8 commits. The successive fallback, metadata, ownership, semantic, and route-test corrections show substantial iteration across interacting mapper rules.
  • Automated suite: 895 passed, 1 skipped. The reported full test run provides strong supplemental coverage even though it does not replace real-workspace proof.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • Post redacted before-and-after clawpatch map output and mapped-state diffs from a representative APEX-shaped workspace.
  • Show a rerun against an already-mapped workspace so feature preservation, regrouping, and stale-state behavior are visible.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR provides tests, CI, build results, and automated review but no after-fix run against a real representative monorepo; add redacted before-and-after CLI output or mapped-state artifacts, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] Route regrouping, ownership reassignment, refreshed project metadata, and confidence changes can churn existing persisted feature identities or findings; the PR does not show an upgrade comparison against an already-mapped workspace.
  • [P1] The 180,000-byte default can omit secondary owned, context, or test files from reviews; unit tests establish deterministic budgeting but not that review quality remains adequate on the motivating workspace.
  • [P1] The branch combines several mapper-policy changes across 19 files, making a real before-and-after artifact important to distinguish intended narrowing from lost coverage.

Maintainer options:

  1. Prove an existing-workspace upgrade (recommended)
    Provide a redacted before-and-after mapping transcript and persisted-state comparison showing which features are preserved, regrouped, or intentionally made stale.
  2. Own the mapper migration
    Merge with explicit maintainer acceptance that route and ownership regrouping may change existing feature identities and review continuity.
  3. Reduce the landing surface
    Pause this branch and split prompt-size enforcement from mapper identity and ownership changes if representative upgrade proof is difficult to produce.

Next step before merge

  • [P1] Keep the draft in human review until the contributor supplies representative real-workspace and upgrade proof and a mapper owner accepts the compatibility boundary.

Maintainer decision needed

  • Question: Is the feature-identity and persisted-state churn from grouping routes, reassigning ownership, refreshing project metadata, and changing confidence levels acceptable as one mapper upgrade?
  • Rationale: Automated tests validate the new rules in isolation, but maintainers must decide whether the combined output migration is an intentional compatibility boundary for existing mapped workspaces.
  • Likely owner: steipete — The mapper architecture provenance makes this person the strongest available owner for deciding its upgrade and feature-identity contract.
  • Options:
    • Require upgrade proof (recommended): Ask for redacted before-and-after map output and state diffs from a previously mapped representative monorepo before accepting the combined behavior changes.
    • Accept mapper churn: Merge after ordinary review while explicitly accepting that existing features and findings may be regrouped or marked stale on the next mapping run.
    • Split the policy changes: Separate prompt budgeting from feature regrouping and project-state refresh so each compatibility effect can be reviewed and proven independently.

Security
Cleared: The diff changes local mapper, configuration, prompt construction, documentation, and tests without adding dependencies, remote execution sources, workflow permissions, secret access, or package lifecycle hooks.

Review details

Best possible solution:

Land the changes only after a representative existing monorepo is mapped before and after the branch, demonstrating bounded prompts, preserved multi-entrypoint metadata, non-overlapping ownership, correct mounted-route grouping, and intentional handling of existing persisted feature state.

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

Yes at the source-fixture level: the added tests encode overlapping generic ownership, rejected entrypoints, package-manager precedence, semantic misclassification, mounted routes, and prompt overflow. No high-confidence real-workspace reproduction transcript is supplied.

Is this the best way to solve the issue?

Unclear. The individual repairs follow existing mapper patterns and are well tested, but a representative upgrade comparison is needed to establish that combining prompt limits, feature regrouping, confidence changes, and project-state refresh is the narrowest safe solution.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 041bf4bfd670.

Label changes

Label changes:

  • add P2: This is a normal-priority mapper correctness improvement with limited CLI blast radius and no evidence of data loss, security bypass, or unusable runtime behavior.
  • add merge-risk: 🚨 compatibility: The branch can regroup persisted features, change ownership and confidence classifications, and refresh stored project metadata when existing workspaces map again.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides tests, CI, build results, and automated review but no after-fix run against a real representative monorepo; add redacted before-and-after CLI output or mapped-state artifacts, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.

Label justifications:

  • P2: This is a normal-priority mapper correctness improvement with limited CLI blast radius and no evidence of data loss, security bypass, or unusable runtime behavior.
  • merge-risk: 🚨 compatibility: The branch can regroup persisted features, change ownership and confidence classifications, and refresh stored project metadata when existing workspaces map again.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides tests, CI, build results, and automated review but no after-fix run against a real representative monorepo; add redacted before-and-after CLI output or mapped-state artifacts, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

What I checked:

  • Automated validation: The PR reports format, typecheck, lint, build, and Vitest validation with 895 tests passed and one skipped; repository checks also completed successfully. (e6643f7f1cc0)
  • Prompt-size control: The branch adds maxPromptBytes to configuration and tests aggregate prompt budgeting, including omission when the first file cannot fit. (src/prompt.ts:141, e6643f7f1cc0)
  • Ownership reconciliation: Generic Node source groups are reconciled against specialized ownership and test assignments to reduce overlapping review slices. (src/mapper.ts:317, e6643f7f1cc0)
  • Entrypoint preservation: Validated provider entrypoints retain their symbol, route, and command metadata while falling back to an owned file if every suggested entrypoint is rejected. (src/agent-mapper.ts:240, e6643f7f1cc0)
  • Route regrouping: Multiple server routes declared in one file and framework are grouped into one feature with multiple entrypoints rather than overlapping per-route features. (src/mappers/node-routes.ts:123, e6643f7f1cc0)
  • Proof gap: The PR body supplies automated checks and automated review results, but no redacted before-and-after clawpatch map output or mapped-state artifact from an actual APEX-shaped workspace. (e6643f7f1cc0)

Likely related people:

  • steipete: The central mapper architecture and broad mapper surface appear to originate from commit cdd58aca13587b939b64687efa3451bec74a1672. (role: introduced behavior; confidence: high; commits: cdd58aca1358; files: src/mapper.ts, src/detect.ts, src/prompt.ts)
  • rohitjavvadi: Recent merged mapper hardening touched the same orchestration, detection, prompt, language-mapper, and regression-test surfaces affected here. (role: recent area contributor; confidence: medium; commits: 71ad2242ce0c; files: src/agent-mapper.ts, src/mapper.ts, src/detect.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 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix(mapper): tighten monorepo review slices This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

@steipete

Copy link
Copy Markdown
Contributor

Closing after live upgrade validation. On an unchanged real monorepo, this branch changed 2,337 of 2,973 existing feature IDs, leaving prior review history attached to skipped records instead of the replacement active features.

The ownership and route-grouping improvements are useful, but this draft combines them with prompt, detection, status, confidence, and agent-metadata changes while making an unbounded persisted-state migration without a defined compatibility contract. Any future work should first preserve or explicitly migrate feature identity/history; the independent prompt and agent-metadata improvements can remain separate.

Thanks for the substantial investigation and implementation work.

@steipete steipete closed this Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants