lhumina_code/hero_shrimp
1
0
Fork 0
Code Issues 28 Pull requests 3 Projects Releases 1 Packages Wiki Activity Actions

ask agent in crew introduce new question twice #72

New issue
Closed
opened 2026-06-02 07:56:16 +00:00 by salmaelsoly · 4 comments
salmaelsoly commented 2026-06-02 07:56:16 +00:00
Member
Copy link

image

![image](/attachments/d26ab5b2-0f38-4fb9-9718-f9cbbe4a6790)
image.png
75 KiB
salmaelsoly removed their assignment 2026-06-03 09:38:31 +00:00
salmaelsoly removed their assignment 2026-06-03 09:38:59 +00:00
salmaelsoly commented 2026-06-03 11:37:47 +00:00
Author
Member
Copy link

Implementation Spec for Issue #72

Objective

Stop the operator's new question from appearing twice as an ASK bubble in the crew agent chat thread when using "Ask this agent". The question must render exactly once from the moment it is sent until (and after) the agent replies.

Root Cause Analysis

This is a UI double-render, not a backend double-insert.

  • The backend persists the ask exactly once. In crates/hero_shrimp_server/src/rpc/methods/crew/mod.rs, method_crew_send builds a single AgentMessageRow and writes it once via runtime.database.upsert_agent_message(&row). No second insert occurs on that path.
  • The frontend renders the ask twice during the in-flight window. In crates/hero_shrimp_web/ui/src/components/CrewPage.tsx:
    • sendAsk() (lines 176-194) optimistically sets pendingAsk and then calls rpc("crew.send", ...) followed by refresh(). refresh() re-fetches crew.messages and repopulates msgs(), so the real persisted ask now appears in byAgent().
    • focusedThread() (lines 204-216) unconditionally appends an optimistic ask bubble (line 211) plus a "working..." reply (line 212) whenever pendingAsk is set for the focused agent.
    • pendingAsk is only cleared when a reply crosses the baseline reply count (lines 196-201: if (replies > p.replies0) setPendingAsk(null)).

Result: after refresh() pulls the real ask into base but before any reply lands, the thread shows both (a) the real ask from byAgent() and (b) the optimistic ask from focusedThread() — the duplicate ASK bubble seen in the screenshot. The optimistic bubble is keyed off "reply count" rather than "is my ask now present in the fetched messages", so it lingers alongside the real ask.

Requirements

  • The optimistic ask / "working..." placeholder must disappear as soon as the operator's real ask is present in the freshly fetched msgs(), not only when a reply arrives.
  • Until the real ask is present, keep showing the optimistic bubbles (preserves the "feels alive" UX the original code intends).
  • No duplicate ASK bubble at any point in the in-flight lifecycle.
  • No backend changes; the ask is already persisted once.

Files to Modify/Create

  • crates/hero_shrimp_web/ui/src/components/CrewPage.tsx - Fix the optimistic-ask clearing/merging logic so the placeholder is suppressed once the real ask is fetched.
  • crates/hero_shrimp_web/static/assets/ (generated) - Rebuilt Vite bundle the server serves; regenerated after the source change.

Implementation Plan

Step 1: Clear the optimistic ask once the real ask is present in fetched messages

Files: crates/hero_shrimp_web/ui/src/components/CrewPage.tsx

  • Augment the createEffect at lines 196-201 so it ALSO clears pendingAsk when the real ask now exists in byAgent()[p.to], matched by kind ask + trimmed text equality against the pending ask.
  • Keep the existing reply-count crossing check as a secondary clear condition (so a fast reply still clears it).
    Dependencies: none

Step 2: Make focusedThread() defensive against the same-text real ask

Files: crates/hero_shrimp_web/ui/src/components/CrewPage.tsx

  • In focusedThread() (lines 204-216), before appending the optimistic ask bubble, guard against the case where the real ask is already in base (a message with kind ask whose trimmed text equals the pending ask text).
  • If the real ask is already present, do not append the optimistic ask. Still append only the "working..." reply when no reply has landed yet, to preserve the working indicator without duplicating the ask.
  • Rationale: a render-time safety net so the ask is never rendered twice even if the Step 1 effect has not yet re-run (effect/timing ordering).
    Dependencies: none (independent of Step 1; both applied for a robust fix)

Step 3: Rebuild the UI bundle

Files: crates/hero_shrimp_web/ui/ -> outputs to crates/hero_shrimp_web/static/assets/

  • Run the UI build so the server serves corrected code (npm run build in crates/hero_shrimp_web/ui/, which runs vite build and writes hashed assets into ../static/assets).
  • Confirm crates/hero_shrimp_web/static/assets/ is regenerated (old hashed app.<hash>.js replaced).
    Dependencies: Steps 1 and 2 complete first.

Acceptance Criteria

  • Sending a new question via "Ask this agent" shows the ASK bubble exactly once immediately after Send.
  • During the in-flight window (after crew.send + refresh() but before the reply), the thread shows the single real ask plus at most one "working..." indicator — no second ASK bubble.
  • After the reply arrives, the thread shows one ASK and one REPLY; no leftover optimistic placeholder.
  • No backend changes required; method_crew_send still persists exactly one row.
  • crates/hero_shrimp_web/static/assets/ rebuilt and committed.

Notes

  • The duplicate is purely client-side timing between the optimistic bubble and the refresh()-fetched real ask; it is not fixed in the backend.
  • Text-equality matching is the pragmatic dedup key because the optimistic bubble has no id. A stronger key (the id returned by crew.send) is possible later but is a larger change; the text-match fix is the minimal, focused solution.
  • The polling cadence logic (anyAskInFlight) is unaffected and left as-is.
## Implementation Spec for Issue #72 ### Objective Stop the operator's new question from appearing twice as an `ASK` bubble in the crew agent chat thread when using "Ask this agent". The question must render exactly once from the moment it is sent until (and after) the agent replies. ### Root Cause Analysis This is a **UI double-render**, not a backend double-insert. - The backend persists the ask exactly once. In `crates/hero_shrimp_server/src/rpc/methods/crew/mod.rs`, `method_crew_send` builds a single `AgentMessageRow` and writes it once via `runtime.database.upsert_agent_message(&row)`. No second insert occurs on that path. - The frontend renders the ask twice during the in-flight window. In `crates/hero_shrimp_web/ui/src/components/CrewPage.tsx`: - `sendAsk()` (lines 176-194) optimistically sets `pendingAsk` and then calls `rpc("crew.send", ...)` followed by `refresh()`. `refresh()` re-fetches `crew.messages` and repopulates `msgs()`, so the **real persisted ask** now appears in `byAgent()`. - `focusedThread()` (lines 204-216) unconditionally appends an **optimistic ask bubble** (line 211) plus a "working..." reply (line 212) whenever `pendingAsk` is set for the focused agent. - `pendingAsk` is only cleared when a **reply** crosses the baseline reply count (lines 196-201: `if (replies > p.replies0) setPendingAsk(null)`). Result: after `refresh()` pulls the real ask into `base` but **before any reply lands**, the thread shows both (a) the real ask from `byAgent()` and (b) the optimistic ask from `focusedThread()` — the duplicate `ASK` bubble seen in the screenshot. The optimistic bubble is keyed off "reply count" rather than "is my ask now present in the fetched messages", so it lingers alongside the real ask. ### Requirements - The optimistic ask / "working..." placeholder must disappear as soon as the operator's real ask is present in the freshly fetched `msgs()`, not only when a reply arrives. - Until the real ask is present, keep showing the optimistic bubbles (preserves the "feels alive" UX the original code intends). - No duplicate `ASK` bubble at any point in the in-flight lifecycle. - No backend changes; the ask is already persisted once. ### Files to Modify/Create - `crates/hero_shrimp_web/ui/src/components/CrewPage.tsx` - Fix the optimistic-ask clearing/merging logic so the placeholder is suppressed once the real ask is fetched. - `crates/hero_shrimp_web/static/assets/` (generated) - Rebuilt Vite bundle the server serves; regenerated after the source change. ### Implementation Plan #### Step 1: Clear the optimistic ask once the real ask is present in fetched messages Files: `crates/hero_shrimp_web/ui/src/components/CrewPage.tsx` - Augment the `createEffect` at lines 196-201 so it ALSO clears `pendingAsk` when the real ask now exists in `byAgent()[p.to]`, matched by kind `ask` + trimmed text equality against the pending ask. - Keep the existing reply-count crossing check as a secondary clear condition (so a fast reply still clears it). Dependencies: none #### Step 2: Make `focusedThread()` defensive against the same-text real ask Files: `crates/hero_shrimp_web/ui/src/components/CrewPage.tsx` - In `focusedThread()` (lines 204-216), before appending the optimistic ask bubble, guard against the case where the real ask is already in `base` (a message with kind `ask` whose trimmed text equals the pending ask text). - If the real ask is already present, do not append the optimistic ask. Still append only the "working..." reply when no reply has landed yet, to preserve the working indicator without duplicating the ask. - Rationale: a render-time safety net so the ask is never rendered twice even if the Step 1 effect has not yet re-run (effect/timing ordering). Dependencies: none (independent of Step 1; both applied for a robust fix) #### Step 3: Rebuild the UI bundle Files: `crates/hero_shrimp_web/ui/` -> outputs to `crates/hero_shrimp_web/static/assets/` - Run the UI build so the server serves corrected code (`npm run build` in `crates/hero_shrimp_web/ui/`, which runs `vite build` and writes hashed assets into `../static/assets`). - Confirm `crates/hero_shrimp_web/static/assets/` is regenerated (old hashed `app.<hash>.js` replaced). Dependencies: Steps 1 and 2 complete first. ### Acceptance Criteria - [ ] Sending a new question via "Ask this agent" shows the `ASK` bubble exactly once immediately after Send. - [ ] During the in-flight window (after `crew.send` + `refresh()` but before the reply), the thread shows the single real ask plus at most one "working..." indicator — no second `ASK` bubble. - [ ] After the reply arrives, the thread shows one `ASK` and one `REPLY`; no leftover optimistic placeholder. - [ ] No backend changes required; `method_crew_send` still persists exactly one row. - [ ] `crates/hero_shrimp_web/static/assets/` rebuilt and committed. ### Notes - The duplicate is purely client-side timing between the optimistic bubble and the `refresh()`-fetched real ask; it is not fixed in the backend. - Text-equality matching is the pragmatic dedup key because the optimistic bubble has no `id`. A stronger key (the `id` returned by `crew.send`) is possible later but is a larger change; the text-match fix is the minimal, focused solution. - The polling cadence logic (`anyAskInFlight`) is unaffected and left as-is.
salmaelsoly commented 2026-06-03 12:39:53 +00:00
Author
Member
Copy link

Test Results

This fix is frontend-only (single SolidJS component plus the regenerated bundle); the backend was not touched, so no Rust tests are in scope.

Production build (vite)

  • Command: npm run build in crates/hero_shrimp_web/ui
  • Result: PASS — 99 modules transformed, bundle emitted to crates/hero_shrimp_web/static/assets/app.D6QRCr1C.js, static/index.html updated to the new hash.

TypeScript typecheck (tsc --noEmit)

  • The changed file CrewPage.tsx reports 0 type errors.
  • tsc reports some pre-existing errors in unrelated, untouched files (ChatActivity.tsx, CostBadge.tsx, DiffViewer.tsx, HeroWelcome.tsx, store.ts). These are present on development independently of this change and do not affect the build pipeline (the build script runs vite build, which does not run tsc).

Summary

  • Total relevant checks: 2 (build, typecheck of changed file)
  • Passed: 2
  • Failed: 0
  • No regressions introduced by this change.
## Test Results This fix is frontend-only (single SolidJS component plus the regenerated bundle); the backend was not touched, so no Rust tests are in scope. ### Production build (vite) - Command: `npm run build` in `crates/hero_shrimp_web/ui` - Result: PASS — 99 modules transformed, bundle emitted to `crates/hero_shrimp_web/static/assets/app.D6QRCr1C.js`, `static/index.html` updated to the new hash. ### TypeScript typecheck (`tsc --noEmit`) - The changed file `CrewPage.tsx` reports **0 type errors**. - `tsc` reports some pre-existing errors in unrelated, untouched files (`ChatActivity.tsx`, `CostBadge.tsx`, `DiffViewer.tsx`, `HeroWelcome.tsx`, `store.ts`). These are present on `development` independently of this change and do not affect the build pipeline (the `build` script runs `vite build`, which does not run `tsc`). ### Summary - Total relevant checks: 2 (build, typecheck of changed file) - Passed: 2 - Failed: 0 - No regressions introduced by this change.
salmaelsoly commented 2026-06-03 12:43:37 +00:00
Author
Member
Copy link

Implementation Summary

Fixed the duplicate ASK bubble that appeared when an operator asks a crew agent a new question via "Ask this agent".

Root cause

Client-side render timing, not a backend double-insert. After sendAsk() optimistically showed the ask, refresh() re-fetched the real persisted ask into the message list, but the optimistic copy was only cleared when a reply arrived — so during the in-flight window both the real ask and the optimistic ask rendered together.

Changes

  • crates/hero_shrimp_web/ui/src/components/CrewPage.tsx
    • The createEffect that clears pendingAsk now also clears it as soon as the real ask is present in the re-fetched messages (matched by kind ask + trimmed text), in addition to the existing reply-count trigger.
    • focusedThread() now guards before appending the optimistic ask: if the real ask is already in the thread it is not duplicated; the "working..." indicator is still shown until a reply lands.
  • crates/hero_shrimp_web/static/index.html and crates/hero_shrimp_web/static/assets/ — regenerated Vite bundle (app.D6QRCr1C.js) reflecting the source change.

No backend changes; method_crew_send still persists exactly one row.

Test results

  • Production build (vite build): PASS.
  • Typecheck of the changed file: 0 errors.
  • No regressions introduced.
## Implementation Summary Fixed the duplicate ASK bubble that appeared when an operator asks a crew agent a new question via "Ask this agent". ### Root cause Client-side render timing, not a backend double-insert. After `sendAsk()` optimistically showed the ask, `refresh()` re-fetched the real persisted ask into the message list, but the optimistic copy was only cleared when a *reply* arrived — so during the in-flight window both the real ask and the optimistic ask rendered together. ### Changes - `crates/hero_shrimp_web/ui/src/components/CrewPage.tsx` - The `createEffect` that clears `pendingAsk` now also clears it as soon as the real ask is present in the re-fetched messages (matched by kind `ask` + trimmed text), in addition to the existing reply-count trigger. - `focusedThread()` now guards before appending the optimistic ask: if the real ask is already in the thread it is not duplicated; the "working..." indicator is still shown until a reply lands. - `crates/hero_shrimp_web/static/index.html` and `crates/hero_shrimp_web/static/assets/` — regenerated Vite bundle (`app.D6QRCr1C.js`) reflecting the source change. No backend changes; `method_crew_send` still persists exactly one row. ### Test results - Production build (`vite build`): PASS. - Typecheck of the changed file: 0 errors. - No regressions introduced.
salmaelsoly commented 2026-06-03 12:51:34 +00:00
Author
Member
Copy link

Pull request opened: #84

This PR implements the changes discussed in this issue.

Pull request opened: https://forge.ourworld.tf/lhumina_code/hero_shrimp/pulls/84 This PR implements the changes discussed in this issue.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
development_unify_budget_limit
integration
development
integration_fix_model_select_boxes
development_freeze
port-from-xmoncode-shrimp
development_structured_incomplete_field
development_kristof
development_comply
latest-main
latest
v2.0.0
v1.0.0
v1.0.0-rc2
Labels
Clear labels   
prio_critical

   
prio_low

   
type_bug

   
type_contact

   
type_issue

   
type_lead

   
type_question

   
type_story

   
type_task
No labels
prio_critical
prio_low
type_bug
type_contact
type_issue
type_lead
type_question
type_story
type_task
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lhumina_code/hero_shrimp#72
No description provided.