feat(plan): outline issue 118 generated channel progress

2026-08-09 18:25:43 +00:00 · 2026-05-31 16:04:20 +07:00
parent 4a907135a6
commit afa32f5fa0
8 changed files with 527 additions and 0 deletions
@@ -0,0 +1,57 @@
+---
+phase: 1
+title: "Characterization Tests"
+status: pending
+priority: P1
+effort: ""
+dependencies: []
+---
+
+# Phase 1: Characterization Tests
+
+## Overview
+
+Lock current behavior before changing semantics. These tests should fail only where issue #118 intentionally changes default quick acknowledgement from fixed templates to generated progress plus fallback.
+
+## Requirements
+
+- Functional: prove current template-only quick ack behavior and `block.reply` delivery gates.
+- Functional: prove existing final dedup depends on delivered `block.reply`.
+- Non-functional: tests must not use external LLM calls or channel network calls.
+
+## Architecture
+
+Use existing unit-test surfaces:
+- `internal/channels/chat_behavior_test.go` for resolver and preview decisions.
+- `internal/channels/chat_behavior_events_test.go` for timer, fallback, streaming, and block reply delivery.
+- Existing pipeline tests in `internal/pipeline/stages_test.go` already cover `block.reply` only when tool calls exist; add only if a behavior gap is discovered.
+
+No DB, migration, or integration fixture is needed.
+
+## Related Code Files
+
+- Modify: `internal/channels/chat_behavior_test.go`
+- Modify: `internal/channels/chat_behavior_events_test.go`
+- Optional modify: `cmd/gateway_consumer_normal_test.go` if a focused final-dedup test already exists nearby
+- Read: `internal/pipeline/think_stage.go`
+- Read: `internal/agent/loop_pipeline_adapter.go`
+
+## Implementation Steps
+
+1. Add resolver tests showing old default template behavior and the intended new default mode contract.
+2. Add event tests for generated progress canceling fallback before fallback timer sends.
+3. Add event tests for fallback sending only when no `block.reply` has been delivered.
+4. Add streaming guard test: generated progress/fallback must not duplicate streaming chunks.
+5. Add compatibility tests for explicit fixed-template mode so users can keep old behavior.
+6. Run the focused Go test package and confirm new tests fail before implementation.
+
+## Success Criteria
+
+- [ ] Tests describe generated-first, template-fallback behavior.
+- [ ] Tests prove no separate LLM call is required.
+- [ ] Tests and review prove this issue adds no new persistence path.
+- [ ] Tests fail before Phase 2/3 implementation and pass after.
+
+## Risk Assessment
+
+Risk: tests accidentally assert impossible "instant" LLM output. Mitigation: state generated progress depends on main-turn `block.reply`, not pre-LLM response.
@@ -0,0 +1,82 @@
+---
+phase: 2
+title: "Config Contract"
+status: pending
+priority: P1
+effort: ""
+dependencies: [1]
+---
+
+# Phase 2: Config Contract
+
+## Overview
+
+Add an explicit quick acknowledgement mode contract so fixed templates stop being the default behavior while old configurations remain representable.
+
+## Requirements
+
+- Functional: default `chat_behavior.quick_ack` mode is generated-first with template fallback.
+- Functional: explicit fixed-template mode preserves old behavior.
+- Functional: existing `templates` field remains the fallback template list.
+- Functional: config preview returns enough metadata for UI to explain generated vs fallback decisions.
+- Non-functional: additive JSON config only; no migration.
+
+## Architecture
+
+Proposed additive shape:
+
+```json
+{
+  "gateway": {
+    "chat_behavior": {
+      "enabled": true,
+      "quick_ack": {
+        "enabled": true,
+        "mode": "llm_generated",
+        "min_delay_ms": 1000,
+        "templates": ["Got it. Working on it..."]
+      }
+    }
+  }
+}
+```
+
+Mode semantics:
+- `llm_generated`: generated `block.reply` is preferred; `templates` are fallback if no generated progress arrives before `min_delay_ms`.
+- `fixed_template`: old behavior; send template after `min_delay_ms` when eligible.
+- `off`: disables chat-behavior quick acknowledgement only; explicit `gateway.block_reply=true` must still preserve existing block reply delivery.
+
+Implementation detail: use string constants in `internal/channels/chat_behavior.go`; keep config structs string-based to avoid custom JSON code.
+
+## Related Code Files
+
+- Modify: `internal/config/config_channels.go`
+- Modify: `internal/channels/chat_behavior.go`
+- Modify: `internal/gateway/methods/chat_behavior.go`
+- Modify: `internal/channels/chat_behavior_test.go`
+- Read: `cmd/gateway_system_config_sync.go`
+
+## Implementation Steps
+
+1. Add `Mode *string json:"mode,omitempty"` to `QuickAckConfig`.
+2. Add `Mode string` to `ResolvedQuickAckConfig`.
+3. Define accepted modes near existing quick ack defaults.
+4. Update `ResolveChatBehavior` to default quick ack mode to `llm_generated` while retaining fallback templates.
+5. Treat nil/empty mode as `llm_generated`, even when legacy configs have `templates`; this is the requested default change. Treat unknown strings as `llm_generated` and test the fallback.
+6. Update `ShouldSendQuickAck` or split it into clearer helpers if needed:
+   - fallback timer eligibility
+   - fixed template delivery eligibility
+   - generated progress delivery eligibility
+7. Update preview response so UI can show whether acknowledgement is generated-first, fixed-template, or off.
+8. Keep `templates` cleaning behavior but document it as fallback templates.
+
+## Success Criteria
+
+- [ ] Existing JSON config remains backward compatible.
+- [ ] Default resolver no longer treats fixed template as the primary quick ack.
+- [ ] Unknown/empty templates still resolve to the fallback default string.
+- [ ] Preview can distinguish generated-first from fixed-template behavior.
+
+## Risk Assessment
+
+Risk: changing default mode breaks users expecting old fixed-template quick ack. Mitigation: expose explicit `fixed_template` mode and keep `templates` field semantics intact.
@@ -0,0 +1,75 @@
+---
+phase: 3
+title: "Runtime Delivery"
+status: pending
+priority: P1
+effort: ""
+dependencies: [1, 2]
+---
+
+# Phase 3: Runtime Delivery
+
+## Overview
+
+Route generated progress through existing `block.reply` channel delivery and use fixed templates only as the fallback path.
+
+## Requirements
+
+- Functional: generated `block.reply` can be delivered for chat behavior even when global `gateway.block_reply` is false.
+- Functional: explicit `gateway.block_reply=true` preserves current full block reply behavior.
+- Functional: fallback template cancels when a generated `block.reply` arrives.
+- Functional: streaming channels skip generated progress/fallback to avoid duplicate chunks.
+- Non-functional: do not store generated progress and do not create another LLM call.
+- Non-functional: do not change existing run timeline recorder behavior.
+
+## Architecture
+
+Current flow:
+- `cmd/gateway_consumer_normal.go:242` resolves global/per-channel `block_reply`.
+- `cmd/gateway_consumer_normal.go:245` resolves `chat_behavior`.
+- `internal/channels/events.go:276` drops `block.reply` unless `RunContext.BlockReplyEnabled` is true.
+- `internal/channels/events.go:292` cancels quick ack when a `block.reply` is delivered.
+
+Target flow:
+- Register runs with both explicit `block_reply` and chat behavior.
+- In channel event handling, allow `block.reply` delivery when either:
+  - explicit block reply is enabled, or
+  - chat behavior quick ack mode is `llm_generated` and non-streaming.
+- In generated-progress mode, mark `blockReplySent` and cancel fallback timer after a generated message is published.
+- Keep final dedup aligned with actual delivered generated progress, not only explicit `gateway.block_reply`.
+
+## Related Code Files
+
+- Modify: `internal/channels/events.go`
+- Modify: `internal/channels/runs.go`
+- Modify: `internal/channels/manager.go`
+- Modify: `cmd/gateway_consumer_normal.go`
+- Modify: `internal/channels/chat_behavior_events_test.go`
+- Read only: `internal/agent/run_timeline_recorder.go`
+- Read: `internal/pipeline/think_stage.go`
+- Read: `internal/pipeline/observe_stage.go`
+- Read: `internal/agent/loop_pipeline_adapter.go`
+
+## Implementation Steps
+
+1. Add runtime helper(s) that answer:
+   - should schedule fallback template?
+   - should deliver generated `block.reply`?
+   - was a generated/fixed interim message delivered and should final dedup run?
+2. Update run registration context if a new resolved flag is needed; avoid shared mutable global state.
+3. Change `AgentEventBlockReply` handling to use generated-progress eligibility as well as explicit block reply.
+4. Keep streaming guard before publishing outbound.
+5. Ensure fallback timer is canceled when generated progress publishes.
+6. Ensure final dedup checks actual delivered interim reply when chat behavior generated mode delivered one.
+7. Keep retry/tool status messages unchanged unless tests show direct conflict.
+
+## Success Criteria
+
+- [ ] Non-streaming channel can receive LLM-generated progress from main-turn `block.reply` with `gateway.block_reply=false`.
+- [ ] Fallback template sends only if no generated progress was delivered before delay.
+- [ ] Streaming channel gets no duplicate progress messages.
+- [ ] Existing explicit block reply behavior and final dedup remain covered.
+
+## Risk Assessment
+
+Risk: generated progress could duplicate final answer if final dedup remains tied to explicit block reply only. Mitigation: base dedup on actual delivered interim reply count/content.
@@ -0,0 +1,69 @@
+---
+phase: 4
+title: "Dashboard Controls"
+status: pending
+priority: P2
+effort: ""
+dependencies: [2, 3]
+---
+
+# Phase 4: Dashboard Controls
+
+## Overview
+
+Update the Behavior UI so the default is generated progress, and fallback templates are presented as fallback, not the main acknowledgement content.
+
+## Requirements
+
+- Functional: global Behavior card can select quick ack mode.
+- Functional: per-channel override schema can override quick ack mode where current quick ack enabled override exists.
+- Functional: UI preview explains generated-first vs fallback/fixed behavior.
+- Non-functional: all new user-facing strings use existing i18n namespace and en/vi/zh locale files.
+- Non-functional: keep component files under the repo's 200-line guidance or split a focused child component.
+
+## Architecture
+
+Current UI:
+- `normalizeChatBehavior` defaults templates to `"Got it. Working on it..."` in `ui/web/src/pages/config/sections/behavior-section.tsx:128`.
+- `BehaviorChatCard` labels templates as quick ack templates in `ui/web/src/pages/config/sections/behavior-chat-card.tsx:115`.
+- Per-channel schema only overrides `chat_behavior.quick_ack.enabled` in `ui/web/src/pages/channels/channel-schemas.ts:31`.
+
+Target UI:
+- Add a mode control with values generated-first, fixed-template, off.
+- Rename template textarea copy to fallback templates.
+- Keep delay field as fallback/fixed delay.
+- Preserve mobile input font size rule: `text-base md:text-sm` on inputs/selects if custom components require class names.
+
+## Related Code Files
+
+- Modify: `ui/web/src/pages/config/sections/behavior-section.tsx`
+- Modify: `ui/web/src/pages/config/sections/behavior-chat-card.tsx`
+- Modify: `ui/web/src/pages/channels/channel-schemas.ts`
+- Modify: `ui/web/src/i18n/locales/en/config.json`
+- Modify: `ui/web/src/i18n/locales/vi/config.json`
+- Modify: `ui/web/src/i18n/locales/zh/config.json`
+- Optional create: a focused child component under `ui/web/src/pages/config/sections/` if `behavior-chat-card.tsx` would exceed 200 lines
+- Modify: `docs/05-channels-messaging.md`
+
+## Implementation Steps
+
+1. Add mode to `ChatBehaviorValues`.
+2. Normalize default mode to `llm_generated`.
+3. Add mode control and copy that explains no extra LLM call.
+4. Rename template labels/hints to fallback template language.
+5. Update preview rendering for generated-first/fallback/fixed states.
+6. Add per-channel schema override for `chat_behavior.quick_ack.mode`.
+7. Add i18n keys to all three config locale files.
+8. Update channel messaging docs with new semantics and explicit no-storage note.
+
+## Success Criteria
+
+- [ ] Dashboard no longer presents fixed template as default immediate response.
+- [ ] Config payload includes `quick_ack.mode` when changed.
+- [ ] Per-channel override can inherit/generated/fixed/off.
+- [ ] All new UI strings are localized.
+- [ ] Component size guidance is respected or consciously documented.
+
+## Risk Assessment
+
+Risk: adding controls could bloat the existing card. Mitigation: split a small quick-ack settings component if the file crosses 200 lines.
@@ -0,0 +1,62 @@
+---
+phase: 5
+title: "Validation and Ship Handoff"
+status: pending
+priority: P1
+effort: ""
+dependencies: [1, 2, 3, 4]
+---
+
+# Phase 5: Validation and Ship Handoff
+
+## Overview
+
+Run focused and broad validation, update plan status/docs, ship a beta PR, run PR review/fix loop, and report back on issue #118.
+
+## Requirements
+
+- Functional: every acceptance criterion in `plan.md` is validated by tests, build, or explicit review.
+- Functional: issue #118 gets plan comment, implementation report, branch/PR link, and label transition.
+- Non-functional: do not push secrets or unrelated changes.
+
+## Architecture
+
+This phase is process-only. It uses the existing GitHub CLI workflow and repo validation commands. Beta shipping targets `dev` because README describes `dev` branch pushes as beta release path.
+
+## Related Code Files
+
+- Modify: plan status files in this plan directory
+- Modify: docs only if implementation changes public channel behavior
+- No runtime files unless review finds actionable issues
+
+## Implementation Steps
+
+1. Run focused Go tests:
+   `go test ./internal/channels ./internal/config ./internal/gateway/methods`
+2. Run SQLite-tag focused tests:
+   `go test -tags sqliteonly ./internal/channels ./internal/config ./internal/gateway/methods`
+3. Run broad compile/static checks:
+   `go build ./...`
+   `go build -tags sqliteonly ./...`
+   `go vet ./...`
+4. Run web validation:
+   `cd ui/web && pnpm test -- --run`
+   `cd ui/web && pnpm build`
+5. Run `git diff --check` and inspect `git status --short`.
+6. Commit and push implementation with `ck:git cp` semantics.
+7. Create beta PR to `dev` and link issue #118.
+8. Run review-pr fix loop until no actionable findings remain.
+9. Comment issue #118 with implementation summary, PR URL, validation status, and final label update.
+
+## Success Criteria
+
+- [ ] All focused tests pass.
+- [ ] PG and SQLite build pass.
+- [ ] Web build passes.
+- [ ] PR exists against `dev`.
+- [ ] Review/fix loop reports approve or no actionable findings.
+- [ ] Issue label moves from `ready to implement` to `ready to ship beta`.
+
+## Risk Assessment
+
+Risk: broad integration/race tests require local services and may be unavailable. Mitigation: run all local deterministic checks; report any unavailable service-backed checks honestly.
@@ -0,0 +1,98 @@
+---
+title: "Issue 118 LLM-Generated Channel Progress"
+description: "TDD plan for digitopvn/goclaw#118: make channel immediate/progress replies LLM-generated through the existing main-turn block.reply path, with fixed quick_ack templates retained only as fallback."
+status: pending
+priority: P2
+branch: "codex/issue-118-llm-generated-progress-messages"
+tags: [issue-118, channels, chat-behavior, block-reply, tdd]
+blockedBy: []
+blocks: []
+created: "2026-05-31T08:55:50.410Z"
+createdBy: "ck:plan"
+source: skill
+---
+
+# Issue 118 LLM-Generated Channel Progress
+
+## Overview
+
+Implement the approved cheapest path for issue #118.
+
+Decision locked by user:
+- No separate LLM call for immediate/progress messages.
+- Use the existing main-turn `block.reply` event as the generated channel progress message.
+- Keep `quick_ack.templates` only as fallback.
+- Do not store progress messages.
+
+Hard product constraint: without a separate LLM call, GoClaw cannot guarantee a natural LLM-generated message before the main model emits content. The generated progress message is available when the main LLM emits assistant content before tool calls; otherwise a configured fixed template fallback may fire after the fallback delay.
+
+Current implementation facts:
+- `QuickAckConfig` has `enabled`, `min_delay_ms`, and `templates` only, so mode/fallback semantics need an additive config field in `internal/config/config_channels.go:11`.
+- `ResolveChatBehavior` currently defaults templates to `"Got it. Working on it..."` and `ShouldSendQuickAck` only checks enabled plus non-streaming in `internal/channels/chat_behavior.go:57` and `internal/channels/chat_behavior.go:140`.
+- `run.started` schedules quick ack immediately via `internal/channels/events.go:42`.
+- `block.reply` channel delivery is currently gated only by resolved `BlockReplyEnabled` in `internal/channels/events.go:276`.
+- Main-turn generated content already emits `block.reply` from tool iterations in `internal/pipeline/think_stage.go:148`, then sanitizes in `internal/agent/loop_pipeline_adapter.go:107`.
+- Final dedup depends on `blockReplyEnabled` in `cmd/gateway_consumer_normal.go:542`.
+- `internal/agent/run_timeline_recorder.go:137` already maps existing `block.reply` events to assistant-message timeline items. This issue must not add new persistence or broaden recorder behavior; existing timeline behavior is out of scope unless tests show a direct regression.
+
+Scope:
+- Backend config contract and resolver.
+- Channel event delivery semantics.
+- Preview API and dashboard controls.
+- Documentation and issue handoff.
+
+Explicitly out of scope:
+- New LLM provider call.
+- DB schema, timeline/archive persistence, or message history storage for progress messages.
+- Changing existing run timeline recorder semantics.
+- Raw chain-of-thought or tool trace exposure.
+- Per-agent prompt rewriting beyond existing main LLM output.
+
+## Phases
+
+| Phase | Name | Status |
+|-------|------|--------|
+| 1 | [Characterization Tests](./phase-01-characterization-tests.md) | Pending |
+| 2 | [Config Contract](./phase-02-config-contract.md) | Pending |
+| 3 | [Runtime Delivery](./phase-03-runtime-delivery.md) | Pending |
+| 4 | [Dashboard Controls](./phase-04-dashboard-controls.md) | Pending |
+| 5 | [Validation and Ship Handoff](./phase-05-validation-and-ship-handoff.md) | Pending |
+
+## Dependencies
+
+- GitHub issue: `digitopvn/goclaw#118`
+- Related existing plan: `../260529-1210-human-like-channel-chat-behavior/plan.md`
+- Existing generated-content source: `internal/pipeline/think_stage.go`, `internal/agent/loop_pipeline_adapter.go`, `internal/pipeline/observe_stage.go`
+- Existing channel runtime: `internal/channels/events.go`, `internal/channels/runs.go`, `cmd/gateway_consumer_normal.go`
+- Existing config/runtime contract: `internal/config/config_channels.go`, `internal/channels/chat_behavior.go`, `internal/gateway/methods/chat_behavior.go`
+- Existing web config UI: `ui/web/src/pages/config/sections/behavior-section.tsx`, `ui/web/src/pages/config/sections/behavior-chat-card.tsx`, `ui/web/src/pages/channels/channel-schemas.ts`
+- Existing docs: `docs/05-channels-messaging.md`
+
+## Acceptance Criteria
+
+- [ ] Default enabled chat behavior prefers LLM-generated progress from main-turn `block.reply`, not fixed templates.
+- [ ] No extra LLM request is introduced.
+- [ ] Fixed templates remain configurable fallback only.
+- [ ] Fallback does not fire after a generated `block.reply` has already been delivered.
+- [ ] Streaming channels still avoid duplicate progress delivery.
+- [ ] Existing explicit `gateway.block_reply` behavior and final dedup continue to work.
+- [ ] Preview API describes generated-vs-fallback behavior without sending messages.
+- [ ] Dashboard config reflects generated default, fallback template semantics, and all new UI text is localized in en/vi/zh.
+- [ ] No new DB/archive/session-history storage is added for progress messages by this issue.
+
+## Validation Commands
+
+```bash
+go test ./internal/channels ./internal/config ./internal/gateway/methods
+go test -tags sqliteonly ./internal/channels ./internal/config ./internal/gateway/methods
+go build ./...
+go build -tags sqliteonly ./...
+go vet ./...
+cd ui/web && pnpm test -- --run
+cd ui/web && pnpm build
+git diff --check
+```
+
+## Open Questions
+
+None. User selected the no-extra-LLM, main-turn `block.reply`, no-progress-storage path.
@@ -0,0 +1,37 @@
+# Issue 118 Plan Redteam
+
+Plan: `plans/260531-1555-issue-118-llm-generated-channel-progress/plan.md`
+Date: 2026-05-31
+
+## Findings
+
+### Fixed
+
+1. Persistence wording was too absolute.
+   - Evidence: `internal/agent/run_timeline_recorder.go:137` already maps existing `block.reply` events to assistant-message timeline items.
+   - Risk: plan could promise "no progress messages are stored" while existing event recorder already handles `block.reply`.
+   - Fix: plan now says this issue adds no new persistence and does not broaden recorder behavior.
+
+2. `off` mode semantics could accidentally disable explicit `gateway.block_reply`.
+   - Evidence: `internal/channels/events.go:276` currently gates block replies by `RunContext.BlockReplyEnabled`, independent from chat behavior.
+   - Risk: quick ack mode `off` could be over-implemented as a global block reply kill switch.
+   - Fix: phase 02 now says `off` only disables chat-behavior quick acknowledgement; explicit `gateway.block_reply=true` must keep working.
+
+3. Legacy nil mode behavior needed a firm decision.
+   - Evidence: current `QuickAckConfig` has no mode in `internal/config/config_channels.go:11`.
+   - Risk: implementation could infer old fixed-template behavior from non-empty `templates`, silently preserving the old default.
+   - Fix: phase 02 now says nil/empty mode resolves to `llm_generated`, even when legacy configs have templates. This is the requested default change.
+
+## Remaining Risks
+
+- Generated progress cannot be guaranteed before tool execution without a separate LLM call. Plan states this explicitly.
+- Fallback template and later generated progress may both be visible in long runs. Implementation must keep this bounded by existing one-ack behavior plus existing `block.reply` semantics.
+- UI work may push `behavior-chat-card.tsx` past 200 lines. Phase 04 calls for a focused split if needed.
+
+## Verdict
+
+Plan is ready to implement after the fixed findings above.
+
+## Open Questions
+
+None.
@@ -0,0 +1,47 @@
+# Issue 118 Plan Validation
+
+Plan: `plans/260531-1555-issue-118-llm-generated-channel-progress/plan.md`
+Date: 2026-05-31
+
+## User Decisions Checked
+
+- No separate LLM call: preserved in overview, phases 01, 03, and validation criteria.
+- Use main-turn `block.reply`: preserved in overview and runtime phase.
+- Do not store progress messages: plan now scopes this as no new persistence or recorder broadening.
+- Templates fallback only: preserved through `llm_generated` mode and `fixed_template` compatibility.
+
+## Code Claims Checked
+
+- `QuickAckConfig` lacks mode today: verified in `internal/config/config_channels.go:11`.
+- Default template is currently fixed: verified in `internal/channels/chat_behavior.go:14` and resolver defaults at `internal/channels/chat_behavior.go:57`.
+- `run.started` schedules quick ack: verified in `internal/channels/events.go:42`.
+- `block.reply` channel delivery is gated by `BlockReplyEnabled`: verified in `internal/channels/events.go:276`.
+- Main LLM content emits `block.reply` on tool iterations: verified in `internal/pipeline/think_stage.go:148`.
+- Sanitization happens before emitting `block.reply`: verified in `internal/agent/loop_pipeline_adapter.go:107`.
+- Final dedup currently keys off explicit block reply enablement: verified in `cmd/gateway_consumer_normal.go:542`.
+- Existing timeline recorder handles `block.reply`: verified in `internal/agent/run_timeline_recorder.go:137`.
+
+## Whole-Plan Consistency Sweep
+
+- No stale claim that generated response is guaranteed "instant".
+- No plan step creates a DB migration or new storage table.
+- No plan step adds a provider or second LLM request.
+- Backend, UI, docs, tests, and ship handoff are represented.
+- Open questions are resolved by the user's selected direction.
+
+## Validation Commands for Implementation
+
+```bash
+go test ./internal/channels ./internal/config ./internal/gateway/methods
+go test -tags sqliteonly ./internal/channels ./internal/config ./internal/gateway/methods
+go build ./...
+go build -tags sqliteonly ./...
+go vet ./...
+cd ui/web && pnpm test -- --run
+cd ui/web && pnpm build
+git diff --check
+```
+
+## Open Questions
+
+None.