Skip to content

fix(commands): give ask/chat their own fresh reply comment - #5077

Merged
JSONbored merged 1 commit into
mainfrom
fix/qa-fresh-reply-comments
Jul 11, 2026
Merged

fix(commands): give ask/chat their own fresh reply comment#5077
JSONbored merged 1 commit into
mainfrom
fix/qa-fresh-reply-comments

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • ask/chat shared AGENT_COMMAND_COMMENT_MARKER with the main PR review panel (AGENT_COMMAND_COMMENT_MARKER === PR_PANEL_COMMENT_MARKER), so a Q&A answer silently overwrote the review verdict (or a prior answer), and the edited comment stayed at its ORIGINAL timeline position -- never near the question that prompted it. Confirmed live on feat(miner-governor): closed-loop discovery re-entry trigger (#2338) #5051: a chat answer posted successfully, then a second question overwrote it, with neither ever visible near either asking comment.
  • ask/chat now post a fresh comment per invocation via createIssueComment -- the same always-fresh pattern already used for approval/closure/pause/resume notices (review-evasion.ts, agent-action-executor.ts) -- instead of sharing the panel slot. The reply links back to the specific triggering comment via its own html_url (already present on the webhook payload, no need to construct it). Every other command's shared, continuously-updated panel is unchanged.
  • createIssueComment (src/github/pr-actions.ts) now also returns html_url (previously only id), so the answer's own permalink is still recorded correctly in github_agent_command_answers.response_url.
  • Separately: generateChatQaAnswer now retries once when the local Ollama model returns a genuinely empty completion (confirmed happening live on the same PR) before reporting empty_chat_answer -- a thrown error (network/auth) is still never retried, only a resolved-but-empty response.

Closes #5076

Test plan

  • npm run typecheck -- clean
  • New tests: retry-once-on-empty (3 cases: recovers, exhausts, never-retries-on-throw) in ai-chat-qa.test.ts; replyingToUrl rendering + phrasing switch in github-commands.test.ts; createIssueComment's html_url extraction in github-pr-actions.test.ts; 3 end-to-end integration tests in queue-5.test.ts (fresh comment per invocation with distinct reply links, the existing PR-panel comment is never PATCHed, dry-run suppresses the live post)
  • npm run test:coverage (full, unsharded) -- 719 files / 14234 tests passed, 0 failures; manually verified every new line/branch in processors.ts, commands.ts, pr-actions.ts, and ai-chat-qa.ts's retry loop shows non-zero hits on both sides via direct lcov.info inspection
  • npm run test:ci (full gate, incl. UI lint/typecheck/test/build, migrations/schema-drift/openapi/docs-drift checks) -- green
  • npm audit --audit-level=moderate -- 0 vulnerabilities

ask/chat shared AGENT_COMMAND_COMMENT_MARKER with the main PR review
panel, so a Q&A answer silently overwrote the review verdict (or a
prior answer) and the edited comment stayed at its original timeline
position -- never near the question that prompted it. A contributor
asking a second question lost the first answer entirely, with no
visible reply anywhere near either question.

ask/chat now post a fresh comment per invocation via createIssueComment
(the same always-fresh pattern already used for approval/closure/
pause/resume notices), linking back to the triggering comment via its
own html_url. Every other command's shared, continuously-updated panel
is unchanged.

Also fixes a separate issue from the same investigation:
generateChatQaAnswer retries once on a genuinely empty completion from
the local Ollama model (confirmed happening live) before reporting
empty_chat_answer; a thrown error is still never retried.

Closes #5076
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.14%. Comparing base (66ed61d) to head (cd2f654).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/services/ai-chat-qa.ts 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5077   +/-   ##
=======================================
  Coverage   94.14%   94.14%           
=======================================
  Files         467      467           
  Lines       39598    39606    +8     
  Branches    14444    14452    +8     
=======================================
+ Hits        37278    37286    +8     
  Misses       1664     1664           
  Partials      656      656           
Flag Coverage Δ
shard-1 46.46% <41.66%> (-0.02%) ⬇️
shard-2 33.50% <66.66%> (-0.35%) ⬇️
shard-3 31.59% <16.66%> (-0.57%) ⬇️
shard-4 32.69% <41.66%> (+0.97%) ⬆️
shard-5 33.20% <0.00%> (-0.10%) ⬇️
shard-6 45.11% <16.66%> (+0.19%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/github/commands.ts 98.29% <100.00%> (+<0.01%) ⬆️
src/github/pr-actions.ts 100.00% <100.00%> (ø)
src/queue/processors.ts 95.46% <100.00%> (+0.03%) ⬆️
src/services/ai-chat-qa.ts 98.73% <75.00%> (-1.27%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 11, 2026
@loopover-orb

loopover-orb Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

Review updated: 2026-07-11 13:05:43 UTC

8 files · 1 AI reviewer · no blockers · readiness 100/100 · CI failing · unstable

🛑 Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): src/github/pr-actions.ts (matched src/github/pr-actions.ts), src/queue/processors.ts (matched src/queue/**), src/services/ai-chat-qa.ts (matched src/services/**).

Review summary
This PR fixes a real, live-confirmed bug (#5076/#5051): ask/chat shared AGENT_COMMAND_COMMENT_MARKER with the PR review panel, so answers silently overwrote the panel or a prior answer and never appeared near the triggering question. The fix correctly routes ask/chat through createIssueComment (a fresh comment per invocation, gated on mentionMode === "live" to preserve dry-run suppression) while leaving every other command's shared-panel path untouched, and plumbs the triggering comment's existing html_url through for a 'replying to' link rather than constructing one. The bonus one-shot retry on a genuinely-empty (not thrown) Ollama completion in ai-chat-qa.ts is correctly scoped — only the resolved-but-empty branch retries, a thrown error still propagates immediately — and is covered by three distinct unit tests (recovers, exhausts, never-retries-on-throw). New integration tests in queue-5.test.ts directly verify the two most important properties: the existing panel comment is never PATCHed, and two invocations produce two distinct comments with distinct reply links.

Nits — 7 non-blocking
  • src/queue/processors.ts: `replyingToUrl = payload.comment?.html_url ?? undefined` falling back to undefined would leave the QA path still posting a brand-new comment while rendering the old 'updated this command response in place' text (src/github/commands.ts's non-replyingToUrl branch) — worth a defensive check even though GitHub always populates `comment.html_url` on `issue_comment` webhooks in practice.
  • src/github/pr-actions.ts: `data.html_url` is typed optional but GitHub's REST API always returns it for a created comment — consider tightening the return type to `{ id: number; html_url: string }` rather than carrying the optional through to callers.
  • codecov/patch flags 91.66% vs the 99% target — the untested edge is likely the `html_url` fallback/undefined branches noted above; worth adding one case for a webhook payload missing `comment.html_url`.
  • src/services/ai-chat-qa.ts:164 reuses the pre-existing `0.1` temperature literal across both retry attempts — no functional issue, just flagged externally as an unexplained magic number that predates this diff.
  • Add a unit test in github-commands.test.ts for `replyingToUrl` undefined-but-isQaCommand scenario to lock in (or deliberately change) the phrasing shown to users in that edge case.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.

CI checks failing

  • codecov/patch — 91.66% of diff hit (target 99.00%)
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #5076
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 47 registered-repo PR(s), 39 merged, 423 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 47 PR(s), 423 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 47 PR(s), 423 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@JSONbored JSONbored self-assigned this Jul 11, 2026
@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 11, 2026
@JSONbored
JSONbored merged commit 99c2429 into main Jul 11, 2026
17 of 18 checks passed
@JSONbored
JSONbored deleted the fix/qa-fresh-reply-comments branch July 11, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@gittensory ask/chat share the PR-panel comment slot: answers overwrite the review verdict and land far from the question

1 participant