Skip to content

fix(check-pending-questions): include body snippet in Discord DM notification - #1861

Merged
liususan091219 merged 4 commits into
sonichi:mainfrom
bassilkhilo-ag2:fix/pending-questions-body-snippet
Jul 16, 2026
Merged

liususan091219 merged 4 commits into
sonichi:mainfrom
bassilkhilo-ag2:fix/pending-questions-body-snippet

Conversation

@bassilkhilo-ag2

Copy link
Copy Markdown
Collaborator

Summary

Pending-question DMs showed only the question title with no actionable context. Bassil's reaction after the notification fired: "this is annoying to user they will say what do I do with this" — the user receives a ping but has no path to action.

  • get_waiting_questions(): extract the first non-empty, non-strikethrough body line as a snippet field on each ## section entry. Bullet-format entries (one-liners with no body) are unaffected — snippet is "" and the q.get("snippet") guard in the DM renderer skips them.
  • notify_discord_dm(): render ↳ <snippet> indented under each question title when a snippet is present.

Before:

⚠️ 1 pending question waiting:

• [2026-06-30] sync-workspace needs vault remote URL

Reply here or edit pending-questions.md on Bassils-MacBook-Pro to resolve.

After:

⚠️ 1 pending question waiting:

• [2026-06-30] sync-workspace needs vault remote URL
  ↳ To enable cross-machine workspace sync (memory, notes, state/cores), `sync-workspace.sh` needs…

Reply here or edit pending-questions.md on Bassils-MacBook-Pro to resolve.

Relates to feedback report ca011cdc filed via /api/feedback.

Test plan

  • Add a ## [test] some question\nAction: run X to fix this entry to pending-questions.md, run python3 src/check-pending-questions.py --force, confirm the proactive-pending-q-*.txt in results/ contains the ↳ Action: run X to fix this line
  • Confirm bullet-format entries (- **[label, ts]** ...) still render without a ↳ line (no body to extract)
  • Confirm existing tests pass: python3 tests/check-pending-questions.test.py

🤖 Generated with Claude Code

https://claude.ai/code/session_01KXQJogmVSdYrKtYX1LwzwT

bassilkhilo-ag2 added a commit to bassilkhilo-ag2/sutando that referenced this pull request Jun 30, 2026
…onichi#1861)

Covers the snippet extraction in get_waiting_questions() and the arrow
rendering in notify_discord_dm() introduced in PR sonichi#1861:
- ## section with body → snippet is first non-empty, non-~~ line
- Strikethrough opener → skipped, next real line wins
- Empty body → snippet is "" (not missing)
- Bullet-format entries → no snippet (one-liners have no body)
- DM output with snippet → "↳ …" line present under title
- DM output without snippet → no "↳" line

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXQJogmVSdYrKtYX1LwzwT
@liususan091219 liususan091219 added the request-change Has an active CHANGES_REQUESTED review label Jul 3, 2026
@liususan091219

Copy link
Copy Markdown
Collaborator

Codex triage findings (detail for the REQUEST_CHANGES review that follows):

Bug in the fix: check-pending-questions.py:115 can render metadata ('Status: unanswered') as the DM snippet when a section lists status before the action line - the DM still lacks the actual question. Add a behavioral test case with a Status-first section. Also +108 lines of custom test harness is heavy for this narrow behavior.

  • Maddy (MBP)

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Codex triage - see my comment above. - Maddy

@john-the-dev john-the-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Clean improvement. Snippet extraction logic is correct — strikethrough skip + 120-char truncation + graceful empty-body handling. Tests cover all the edge cases I'd want. The ↳ display in Discord DMs will make pending-question notifications actually actionable.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated mergeability review (2026-07-09)

Signal: NOT MERGEABLE

Current blocker(s):

  • Existing review decision is CHANGES_REQUESTED; requested changes need to be resolved.
  • Branch is behind current main; update/merge main and rerun checks before merging.

This is a conservative backlog triage signal. After the blockers are cleared, please rerun checks and request a focused review.

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Triage change-request (owner-approved batch, 2026-07-12 sweep). Conditions found on the current diff:

  • incomplete fix — Skip status metadata; current snippet can DM "Status: unanswered" as context.

Justification: kb triage standards (source-grep test standard, voice/vision live-test policy, necessity/coverage checks) — see sonichi/sutando-pr-triage kb/.
Address the asks and push; re-request review to clear this. If a finding is wrong, reply with the counter-evidence and it will be withdrawn.

cr-reason: incomplete_fix

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Triage change-request (owner-approved batch, 2026-07-12 sweep). Conditions found on the current diff:

  • incomplete fix — Skip status metadata; current snippet can DM "Status: unanswered" as context.

Justification: kb triage standards (source-grep test standard, voice/vision live-test policy, necessity/coverage checks) — see sonichi/sutando-pr-triage kb/.
Address the asks and push; re-request review to clear this. If a finding is wrong, reply with the counter-evidence and it will be withdrawn.

cr-reason: incomplete_fix

@liususan091219

Copy link
Copy Markdown
Collaborator

Daily reminder — this PR has an open change-request from 2026-07-03 (9d) with no commits since. Push fixes or reply to the review to reset the clock; ~2 weeks of silence routes to final-notice and then close-as-abandoned per the triage policy.

nudge: cr-unaddressed day9

@liususan091219 liususan091219 added bug Something isn't working live-test-requested A change-request asks for live-test/repro evidence (voice/vision/runtime policy) and removed bug Something isn't working labels Jul 13, 2026
@liususan091219

Copy link
Copy Markdown
Collaborator

Daily reminder — this PR has an open change-request from 2026-07-03 (11d) with no commits since. Push fixes or reply to the review to reset the clock; ~2 weeks of silence routes to final-notice and then close-as-abandoned per the triage policy.

nudge: cr-unaddressed day11

@liususan091219

Copy link
Copy Markdown
Collaborator

Daily reminder — this PR has an open change-request from 2026-07-03 (12d) with no commits since. Push fixes or reply to the review to reset the clock; ~2 weeks of silence routes to final-notice and then close-as-abandoned per the triage policy.

nudge: cr-unaddressed day12

bassilkhilo-ag2 and others added 3 commits July 16, 2026 13:01
…fication

Pending-question DMs showed "⚠️ N pending question(s) waiting: • title"
with no context about what to do. Users receiving the notification had no
actionable path — they didn't know what the question was asking or how to
answer it ("what do I do with this?" UX complaint, feedback ca011cdc).

Add a one-line snippet to each ## section entry: the first non-empty,
non-strikethrough line of the question body. Displayed as "↳ ..." under
the title in the Discord DM. Bullet-format entries (one-liners) carry no
body and are unaffected — snippet is optional, guarded by q.get("snippet").

Before:
  ⚠️ 1 pending question waiting:
  • [2026-06-30] sync-workspace needs vault remote URL
  Reply here or edit pending-questions.md on Bassils-MacBook-Pro…

After:
  ⚠️ 1 pending question waiting:
  • [2026-06-30] sync-workspace needs vault remote URL
    ↳ To enable cross-machine workspace sync (memory, notes, state/cores)…
  Reply here or edit pending-questions.md on Bassils-MacBook-Pro…

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXQJogmVSdYrKtYX1LwzwT
…onichi#1861)

Covers the snippet extraction in get_waiting_questions() and the arrow
rendering in notify_discord_dm() introduced in PR sonichi#1861:
- ## section with body → snippet is first non-empty, non-~~ line
- Strikethrough opener → skipped, next real line wins
- Empty body → snippet is "" (not missing)
- Bullet-format entries → no snippet (one-liners have no body)
- DM output with snippet → "↳ …" line present under title
- DM output without snippet → no "↳" line

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXQJogmVSdYrKtYX1LwzwT
…DM snippet

Reviewer finding (liususan091219, 2026-07-12 triage): a section whose
**Status:** line comes before its narrative text would DM the status
marker itself as the "action hint" — telling the user nothing they
don't already know from the ping. Skip status-metadata lines the same
way strikethrough lines are already skipped.

Also rebased onto current main (cherry-picked the original two commits
fresh — was 100+ commits behind).
@bassilkhilo-ag2
bassilkhilo-ag2 force-pushed the fix/pending-questions-body-snippet branch from 98a5d65 to fd00ab2 Compare July 16, 2026 17:04
@bassilkhilo-ag2

Copy link
Copy Markdown
Collaborator Author

@liususan091219 — good catch, fixed. Snippet extraction now also skips lines that are just `Status:` metadata (same treatment as the existing strikethrough skip), so a section that writes its Status line before the narrative text no longer DMs "Status: unanswered" as the action hint. Added a regression test (`check-pending-questions-snippet.test.py`, cases "status line skipped" / "status line never appears as the snippet").

Also rebased onto current `main` (was ~100+ commits behind) via a clean cherry-pick of the original two commits. Re-requesting review.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 49%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • src/check-pending-questions.py (100%)

Summary

  • Total: 5 lines
  • Missing: 0 lines
  • Coverage: 100%

@liususan091219

Copy link
Copy Markdown
Collaborator

Maintainer-run live test — PASS — and clearing the live-test-requested label.

Ran this PR's src/check-pending-questions.py (head 17:04 commit, which also addresses the 7/12 incomplete_fix CR) live on the maintainer install with --force: it fired the real notification path end-to-end — macOS notification + the Discord DM through the proactive bridge, listing the 7 real pending questions with body snippets (the owner received that DM directly, ~17:35 — that message IS the evidence). The snippet extraction skipped **Status:** metadata lines exactly as the 17:04 fix intends (the maintainer's real pending-questions.md contains such lines, so the fix path was genuinely exercised).

Context for the label (applied 2026-07-14 15:42 in a batch that regrettably shipped without explanations): DM-notification formatting is user-visible runtime behavior, so the policy asks for a real delivery run rather than unit tests alone. That evidence now exists (above), and the CR's code ask is addressed on head — removing live-test-requested; the re-review loop will re-judge the CR normally.

@liususan091219 liususan091219 removed the live-test-requested A change-request asks for live-test/repro evidence (voice/vision/runtime policy) label Jul 16, 2026
@liususan091219

Copy link
Copy Markdown
Collaborator

Before/after — real Terminal screen capture (pixels untouched; hostname/path redaction done via the visible sed pipe in the command line; fixture pending-question used instead of the owner's real ones, deliberately containing a **Status:** unanswered metadata line to exercise the 17:04 fix):

before/after DM body — main vs PR head

BEFORE (main): the DM lists only the question title — no body snippet.
AFTER (this PR's head): the DM includes the body snippet (└ Need a decision between staging and prod…) and the **Status:** metadata line is correctly skipped.

Together with the live end-to-end delivery run earlier (real DM through the bridge), this completes the evidence: snippet feature works, Status-leak fix works, delivery path works.

@liususan091219

Copy link
Copy Markdown
Collaborator

And the user-facing surface itself — the DM as actually rendered in the Discord client (this PR's code, delivered through the real proactive bridge; fixture question, screenshot cropped to the message):

discord DM rendering — snippet visible

Title line + the ↳ body snippet rendering exactly as the PR intends, **Status:** metadata absent. Evidence set for this PR is now: real end-to-end delivery run, terminal before/after (main vs head), and the rendered client surface.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

LGTM. I ran python3 tests/check-pending-questions-snippet.test.py on the current head; all 10 checks passed, including the regression where a leading **Status:** unanswered line must not become the DM snippet. I also spot-checked src/check-pending-questions.py to confirm the notifier still skips resolved sections before snippet extraction and now only lifts the first non-empty, non-status body line into the Discord DM.

Qingyun's Personal Codex Agent

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

APPROVE - claim verified live against the full evidence standard: terminal before/after (main: title-only DM vs head: body snippet with Status metadata skipped), real end-to-end delivery through the proactive bridge on the real pending-questions file, and the rendered Discord client surface. The 7/12 incomplete_fix CR is addressed on head (17:04 commit) and verified in the same runs. Edge cases covered by the PR's unit tests.

@github-actions

Copy link
Copy Markdown
Contributor

@cla-assistant check

@liususan091219
liususan091219 merged commit 0390212 into sonichi:main Jul 16, 2026
10 checks passed
yixuan-ag2 pushed a commit that referenced this pull request Aug 27, 2026
#1861 asserted bullets carry no snippet because at the time they were
one-liners whose label was the content. The format now carries the ask
after the label, so that assertion pinned a stale fact rather than a
design decision. A label-only bullet still yields no snippet.
yixuan-ag2 added a commit that referenced this pull request Aug 27, 2026
#3444)

* fix(questions): a bullet question delivers its ask, not just its label

The proactive DM renders `snippet`; the bullet parser hardcoded it to the
empty string, so a question carrying options, a recommendation and a default
reached the owner as a bracketed slug and a date. The body was parsed
correctly all along -- nothing rendered it.

* test(questions): the bullet no-snippet premise no longer holds

#1861 asserted bullets carry no snippet because at the time they were
one-liners whose label was the content. The format now carries the ask
after the label, so that assertion pinned a stale fact rather than a
design decision. A label-only bullet still yields no snippet.

---------

Co-authored-by: Yixuan Zhai <yxz.ucd@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

request-change Has an active CHANGES_REQUESTED review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants