Skip to content

feat(search): attribute settled manual ClawHub web searches - #3631

Draft
Patrick-Erichsen wants to merge 2 commits into
factory/claw-764-search-observationsfrom
codex/claw-724-web-capture
Draft

feat(search): attribute settled manual ClawHub web searches#3631
Patrick-Erichsen wants to merge 2 commits into
factory/claw-764-search-observationsfrom
codex/claw-724-web-capture

Conversation

@Patrick-Erichsen

@Patrick-Erichsen Patrick-Erichsen commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Attribute settled manual ClawHub web searches

Problem

Separate deliberate search demand from URL loads, crawlers, and automatic refreshes.

Change

Track transient intent in four search controls. Mark settled or submitted manual searches once; URL loads, filter-only changes, pagination, automatic retries, and canceled-before-dispatch input stay unmarked.

Validation

Inspected desktop/mobile baseline and candidate proof: 10 marked responses matched 10 observations, plus three clear/retype regression observations. Negative cases stayed at zero. Evidence includes 49 screenshots and five recordings.

Review status

Draft for human review. This is a dedicated search-intelligence stack, independent of the plugin-page/category work. No merge or production deployment is authorized.

The full integrated stack at ca843a963a passed bun run ci:unit (6,582 passed, 3 skipped), bun run ci:types-build, package/HTTP/browser checks, and the non-audit static constituents. bun run ci:static remains red on seven dependency advisories in the unchanged baseline lockfile; that repair is deliberately outside this stack. These results are from the saved reviewed head, not a claim that CI on this PR has passed.

The browser proof used a real local ClawHub app with synthetic fixtures and real Convex backend, not generated screenshots. Late client cancellation cannot retract an observation already completed at the backend.

Real browser proof

Before: manual search fixture

After: same visible results with bounded attribution

Related draft PRs

Review ClawHub bottom-to-top: captureweb attributiondashboard/API/CLIweekly producer.

Separate-repository companions: OpenClaw Control UI and Hermit receiver. The producer requires the Hermit receiver; the Control UI source marker requires the capture boundary. No plugin-page/category stack branches are included.

@clawsweeper

clawsweeper Bot commented Sep 8, 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.

@vercel

vercel Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
clawhub Ready Ready Preview Sep 8, 2026 11:15pm UTC

Request Review

@clawsweeper

clawsweeper Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Codex review: blocked before merge. Reviewed September 8, 2026, 7:19 PM ET / 23:19 UTC.

ClawSweeper review

What this changes

The PR attributes manual plugin searches across four web controls, excludes automatic requests, and displays plugin-search failures explicitly.

Merge readiness

Blocked before merge - 4 items remain

This remains distinct from current main, but source review identified two correctness defects. The collaborator-authored draft also explicitly reserves human review.

Priority: P2
Reviewed head: d83e9565133fecf5f91dcf5855b438ea15305f23

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Useful, scoped implementation with substantial supplemental tests, but two source-proven defects and incomplete inspectable runtime evidence remain.
Proof confidence 🦐 gold shrimp (3/6) Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality 🦐 gold shrimp (3/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.
Evidence reviewed 12 items Applicable repository policy: Read the complete root AGENTS.md and ClawHub PR maintainer skill. No nested AGENTS.md files were found under the changed src or specs paths, and no maintainer-notes directory exists.
Verified introduced change: Inspected the pinned base-to-head changes and surrounding callers, adapter, routes, and tests. The delta contains 11 files, with 574 additions and 20 deletions; backend capture belongs to the base layer rather than this PR.
Current-main necessity: Current main’s unified search issues ordinary limit-plus-one plugin requests without manual-intent handling. Its package adapter also lacks searchSource, so the central capability is not already implemented.
Findings 2 actionable findings [P2] Synchronize the page’s intent with subsequent header handoffs
[P2] Render successful results before awaiting the pagination probe
Security None None.

How this fits together

ClawHub’s web search controls fetch plugin catalog results through the HTTP API. Transient manual-search intent adds an attribution marker that the separate capture layer uses to record bounded demand observations.

flowchart TD
  A[Manual search controls] --> B[Consume transient intent]
  B --> C[Plugin search API]
  D[Automatic searches and pagination] --> C
  C --> E[Visible catalog results]
  C --> F[Bounded demand observations]
  E --> G[Browser results]
Loading

Before merge

  • Synchronize the page’s intent with subsequent header handoffs (P2) - Start on /search, submit a header query, wait for its attributed search, then submit the unchanged query in the full-page control. Query navigation preserves this component, so the ref initialized here still contains its original null or older intent even though route context received the header intent. handleSearch creates another unconsumed object and counts the same settled query again. Adopt incoming handoff intents without resetting their consumed state, and cover a handoff into an already-mounted page.
  • Render successful results before awaiting the pagination probe (P2) - When a marked response fills the page, this await prevents its promise from settling until the optional second request finishes. The outer Promise.allSettled therefore keeps all results behind the loading skeleton if that probe is slow or stalls, even though the visible search already succeeded. The adapter has no deadline, and catching rejection does not address a pending request. Publish the successful rows first and update pagination independently, with stale-request protection.
  • Resolve merge risk (P1) - Observation persistence depends on the still-open capture layer at feat(search): capture bounded manual plugin-search observations #3630; this web layer alone does not complete the feature.
  • Complete next step (P2) - Correct both findings, attach inspectable request/observation evidence, and obtain the requested human review with the capture dependency accounted for before landing.

Findings

  • [P2] Synchronize the page’s intent with subsequent header handoffs — src/routes/search.tsx:92
  • [P2] Render successful results before awaiting the pagination probe — src/lib/useUnifiedSearch.ts:303-308
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch size +574/-20 across 11 files The patch spans four search controls and their shared request handling.
Production and test growth Production +178/-20; tests +336/-0; specs +60/-0 Production growth supports transient attribution and error handling, with a dedicated route-level test suite.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Preserve one consumed intent across header/page handoffs and render successful results independently of optional pagination probes.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Preserve one consumed intent across header/page handoffs and render successful results independently of optional pagination probes.

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

Yes, source establishes focused PR-head checks: submit an unchanged full-page query after a header handoff, and delay the optional pagination probe after a successful marked response. These paths were not executed during this read-only review.

Is this the best way to solve the issue?

The control-level attribution approach is appropriate; marking every adapter request would also count automatic traffic. The implementation needs shared handoff state and a pagination probe that cannot block visible results.

Full review comments:

  • [P2] Synchronize the page’s intent with subsequent header handoffs — src/routes/search.tsx:92
    Start on /search, submit a header query, wait for its attributed search, then submit the unchanged query in the full-page control. Query navigation preserves this component, so the ref initialized here still contains its original null or older intent even though route context received the header intent. handleSearch creates another unconsumed object and counts the same settled query again. Adopt incoming handoff intents without resetting their consumed state, and cover a handoff into an already-mounted page.
    Confidence: 0.96
  • [P2] Render successful results before awaiting the pagination probe — src/lib/useUnifiedSearch.ts:303-308
    When a marked response fills the page, this await prevents its promise from settling until the optional second request finishes. The outer Promise.allSettled therefore keeps all results behind the loading skeleton if that probe is slow or stalls, even though the visible search already succeeded. The adapter has no deadline, and catching rejection does not address a pending request. Publish the successful rows first and update pagination independently, with stale-request protection.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.95

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add P2: This bounded search improvement has attribution and loading defects without evidence of an urgent production outage.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.

Label justifications:

  • P2: This bounded search improvement has attribution and loading defects without evidence of an urgent production outage.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.

Evidence

What I checked:

  • Applicable repository policy: Read the complete root AGENTS.md and ClawHub PR maintainer skill. No nested AGENTS.md files were found under the changed src or specs paths, and no maintainer-notes directory exists. (AGENTS.md:1, d83e9565133f)
  • Verified introduced change: Inspected the pinned base-to-head changes and surrounding callers, adapter, routes, and tests. The delta contains 11 files, with 574 additions and 20 deletions; backend capture belongs to the base layer rather than this PR. (d83e9565133f)
  • Current-main necessity: Current main’s unified search issues ordinary limit-plus-one plugin requests without manual-intent handling. Its package adapter also lacks searchSource, so the central capability is not already implemented. (src/lib/useUnifiedSearch.ts:274, 651a7641a204)
  • Release check: The latest published release remains v0.23.3. Historical release-blob inspection was unavailable; no shipped implementation or fixed release is claimed. (87ca030c30f3)
  • Header handoff state: The page initializes lastManualSearchRef only on mount, while incoming header navigation updates route context. A subsequent page submit can therefore create a fresh unconsumed intent for an already-counted query. (src/routes/search.tsx:92, d83e9565133f)
  • Verified route reuse contract: The target directly imports TanStack Router and pins react-router 1.170.29. That version’s MatchInner supplies a component key only through remountDeps; ClawHub configures neither route nor default remount dependencies. Query navigation therefore preserves the page’s ref. (packages/react-router/src/Match.tsx:191, 11dfc9366982)

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)
  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Preserve incoming header intent on the mounted search page and add an unchanged-query resubmit regression.
  • Show completed search results while a delayed or failed pagination probe remains independent.
  • Attach the existing real-browser request/observation pairing with source and authoritative counts; redact private details. Updating the PR body should trigger review, or a maintainer can comment @clawsweeper re-review.

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.

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

Labels

P2 Normal backlog priority with limited blast radius. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant