Skip to content

feat(slack): ask for a thumbs up/down on completion replies in pilot channels - #153

Closed
anomalogravity[bot] wants to merge 1 commit into
prodfrom
grav-715-score-ask
Closed

feat(slack): ask for a thumbs up/down on completion replies in pilot channels#153
anomalogravity[bot] wants to merge 1 commit into
prodfrom
grav-715-score-ask

Conversation

@anomalogravity

Copy link
Copy Markdown

What type of PR is this?

/kind feature

What this PR does / why we need it:

Gravity's Slack replies end with a Task: footer but no invitation to rate the answer. For the agent-effectiveness scorecard (GRAV-715), Slack verdicts only accumulate once the ask exists — so this ships the ask early, pilot-scoped.

  • Completion replies (succeeded/failed transitions) in the score pilot channels now carry a one-line ask, React 👍/👎 to score this., as a context block directly above the existing Task: footer. It never appears on the "working on your request" (accepted) message or on progress/activity updates.
  • Both reactions are pre-seeded on the completion reply in the pilot channels, so a human taps once instead of opening the emoji picker. The emoji names (+1/-1) match the collector's positive/negative lists, which ignore bot reactions, so pre-seeding cannot skew the verdict ratio.
  • reactions.add failures are logged and never block task reporting — the reply itself is already posted, and a missing reaction only costs the one-tap convenience. Added TestSlackTaskReporter_ScoreReactionFailureStillReports for that path.
  • Pilot channels are configurable via the SLACK_SCORE_PILOT_CHANNELS env var (chart value slackServer.scorePilotChannels). Empty/unset disables the feature everywhere, so default deployments are unchanged.

Which issue(s) this PR is related to:

N/A (Gravity Linear GRAV-715 — internal tracker, not a GH issue)

Special notes for your reviewer:

  • FormatSlackTransitionMessage keeps its public signature (ask off); the ask is threaded through the new formatSlackTransitionMessage(..., askForScore) core so the split-message math reserves the extra block correctly and the ask rides on the last part (which carries the trailing blocks).
  • In the in-place-update path (progress message edited on completion), the reactions land on the message that actually carries the ask — the last posted reply (or the updated progress message when it fits in one).
  • Unit-tested in slack_test.go (ask placement per phase, survives splitting, ParseScorePilotChannels matrix) and watcher_test.go (pilot vs. non-pilot, nil map, failed phase, update path, reaction-failure resilience).

Does this PR introduce a user-facing change?

Gravity's Slack completion replies in the configured pilot channels now ask "React 👍/👎 to score this." with both reactions pre-seeded for one-tap scoring.

…channels

Completion replies in the score pilot channels now carry a one-line ask,
'React 👍/👎 to score this.', right above the existing Task: footer, and
both reactions are pre-seeded on the reply so a human taps once instead
of opening the emoji picker.

The ask only appears on terminal (succeeded/failed) transitions, and only
in the channels named in the new SLACK_SCORE_PILOT_CHANNELS env var
(chart value slackServer.scorePilotChannels); an empty value keeps the
feature off everywhere. Reaction names (+1/-1) match the collector's
lists, which ignore bot reactions, so pre-seeding does not skew the
verdict ratio. reactions.add failures are logged and never block task
reporting.
@anomalogravity

Copy link
Copy Markdown
Author

@greptile review

@greptile-apps

greptile-apps Bot commented Aug 30, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a pilot-scoped Slack completion-rating prompt and pre-seeds positive and negative reactions, with chart configuration for selecting pilot channels.

  • Adds score-ask formatting to terminal Slack replies while preserving the existing public formatter behavior.
  • Adds reaction support and targets the final completion message in normal and progress-update reporting paths.
  • Wires slackServer.scorePilotChannels through the chart and environment into the Slack reporter.
  • Adds formatting, configuration, phase, update-path, and reaction-failure tests.

Confidence Score: 4/5

This PR should not merge until partial continuation failures stop persisting an incomplete completion reply and attaching score reactions to the wrong message.

A failed final continuation in the progress-update path is ignored, after which reactions are applied to an earlier fragment and the terminal phase is persisted, preventing delivery from being retried.

Files Needing Attention: internal/reporting/watcher.go

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Phase[Task reaches succeeded or failed] --> Pilot{Channel in score pilot?}
  Pilot -- No --> Normal[Post normal completion reply]
  Pilot -- Yes --> Format[Append score ask before Task footer]
  Format --> Split{Reply split?}
  Split -- No --> Post[Post or update completion message]
  Split -- Yes --> Parts[Post continuation parts]
  Parts --> Last[Final part carries ask and footer]
  Post --> React[Add +1 and -1 reactions]
  Last --> React
  React --> Persist[Persist reported terminal phase]
Loading

Fix all with Greploop Fix All in Claude Code

Prompt To Fix All With AI
### Issue 1
internal/reporting/watcher.go:598-603
**Failed continuation persists completion**

When the final ask-bearing continuation fails after an earlier continuation succeeds, this loop retains the earlier reply timestamp, seeds reactions on that fragment, and persists the terminal phase. The score ask and Task footer are never delivered, and later reporting cycles cannot retry the incomplete reply.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "feat(slack): ask for a thumbs up/down on..." | Re-trigger Greptile

Comment on lines 598 to +603
log.Error(err, "Failed to post continuation message", "task", task.Name)
continue
}
lastReplyTS = replyTS
}
tr.preSeedScoreReactions(ctx, channel, lastReplyTS, askForScore)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Failed continuation persists completion

When the final ask-bearing continuation fails after an earlier continuation succeeds, this loop retains the earlier reply timestamp, seeds reactions on that fragment, and persists the terminal phase. The score ask and Task footer are never delivered, and later reporting cycles cannot retry the incomplete reply.

Knowledge Base Used: Deployment and operational interfaces

Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/reporting/watcher.go
Line: 598-603

Comment:
**Failed continuation persists completion**

When the final ask-bearing continuation fails after an earlier continuation succeeds, this loop retains the earlier reply timestamp, seeds reactions on that fragment, and persists the terminal phase. The score ask and Task footer are never delivered, and later reporting cycles cannot retry the incomplete reply.

**Knowledge Base Used:** [Deployment and operational interfaces](https://app.greptile.com/anomalo/-/custom-context/knowledge-base/datagravity-ai/kelos/-/docs/deployment-and-operations.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code

@knechtionscoding

Copy link
Copy Markdown
Collaborator

Closing in favor of doing this entirely on the dquality side — no Kelos change needed. Tracked in GRAV-715.

The reaction pre-seeding and channel gating here can both be done from a job we own: reactions.add against Gravity's final replies (slack_react.py already does this and already treats already_reacted as success, so it is idempotent), with the pilot channel list as our own config.

The one thing that genuinely requires this PR is the ask text line, since Kelos composes the message body. We are choosing to drop it: a pre-seeded 👍/👎 pair is self-explanatory on its own, and it costs no extra sentence on every reply.

The deciding factor is that this targets prod rather than upstream/main, so it would be a 555-line carry-patch on a fork already 84 commits behind upstream — re-applied on every rebase, indefinitely — and it would keep the feature waiting on the unresolved prod-landing question.

Nothing here is wasted if we change our minds: the branch stays, and the trade-off is written up in GRAV-715. What we give up is instant placement (reactions now appear on the next collector tick, within 5 minutes) and the text line.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant