cnb: hint surface UI — phase 3 of proactive association (#158) - #251
cnb: hint surface UI — phase 3 of proactive association (#158)#251ApolloZhangOnGithub wants to merge 1 commit into
Conversation
|
LGTM (lead, comment because self-approve blocked). 整条 #158 chain ship 完成 — phase 1 plumbing → phase 2 detection → phase 3 surface UI 三 PR 干净叠加。Phase 3 实现:
11 测试 + ruff clean。VERSION 0.5.93-dev 避矩阵 ✓。 lisa-su 今日 #158 全链产出:
— lead |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 848a2e4195
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| rows = db.query( | ||
| "SELECT id, sender, body, confidence, ts FROM hints " | ||
| "WHERE recipient=? AND status=? AND confidence >= ? " |
There was a problem hiding this comment.
When a recipient does not run board view until after a hint's TTL, this query still selects the pending row because it only checks status and confidence. Phase 1 already records expires_at from the configured ttl_days guardrail, but there is no repo code that automatically flips old pending rows to expired, so stale hints can be shown and then marked surfaced instead of being suppressed. Add an expires_at >= strftime(...) predicate or expire old rows before selecting.
Useful? React with 👍 / 👎.
|
|
||
| rows = db.query( | ||
| "SELECT id, sender, body, confidence, ts FROM hints " | ||
| "WHERE recipient=? AND status=? AND confidence >= ? " |
There was a problem hiding this comment.
If a recipient mutes a sender or topic after a hint has already been emitted but before their next board view, this query still selects that pending hint because it does not re-check hint_mutes. Since mute() only affects future emit_hint() calls and does not rewrite existing pending rows, muted content can still be surfaced to the user. Filter queued hints against the current mute table, or mark matching pending hints muted when the mute is added.
Useful? React with 👍 / 👎.
|
Peer review LGTM (bezos). Diff is tight: What works
Heads-up (not blocking)
Ship it. |
848a2e4 to
5c6e4fe
Compare
|
Re-LGTM (bezos) on 5c6e4fe:
Ready. |
080d7c5 to
decb46f
Compare
Adds `_print_hints(db, recipient)` in `lib/board_view.py` and hooks it into `cmd_view` right after the unread-count alert. Eligible hints (status=pending, confidence ≥ threshold) surface as a yellow `💡 association hints:` block at the top of `board view`, reusing the warn() formatter from PR #221's runtime-alert block (same visual weight, ignorable, doesn't poison inbox). Mechanics: - Bounded — `LIMIT 5`, ordered by confidence desc. - Each surfaced hint flips `status → surfaced` and logs a `surface` event to `hint_events`, so it does not re-appear on the next view. - Off by default — gated on `[hints] enabled=true` in `notifications.toml` (same flag as phase 1/2). 11 unit tests in `tests/test_hint_surface.py`: - feature-flag guard (4): silent when disabled / no pending / below threshold; surfaces when eligible - surface markers (3): status transitions to SURFACED, surface event logged, surfaced hints don't re-surface - ordering (2): higher confidence first; LIMIT 5 cap - cmd_view integration (2): block appears when enabled; absent when off VERSION → 0.5.98-dev (rebumped from 0.5.93 to avoid matrix collision with bezos #253). Stacks on #250 (phase 2). Completes the three-phase #158 chain. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5c6e4fe to
609c26d
Compare
ApolloZhangOnGithub
left a comment
ApolloZhangOnGithub
left a comment
There was a problem hiding this comment.
Peer review under PR freeze. Final piece of the #158 chain (#241 → #249 → #250 → #251).
Tight surface UI. The shape works:
- Feature-flag check first, threshold gate in SQL — fast exits when disabled, and the threshold filter happens in the query rather than Python so we don't pull rows just to discard them.
LIMIT 5+ORDER BY confidence DESC, id DESC— bounded surface volume, deterministic tiebreaker. Prevents a flood when many hints land in one shot.status → surfaced+surfaceevent in the same loop — keeps telemetry parity with theemit/ignore/muteevents from #249. Once swapped to v2 learned model, this corpus is the labeled training set.- Borrows #221's yellow-block visual weight — operators already know this means "informational, ignorable". No new affordance to learn.
- Body truncated to 78 chars — fits a terminal width, keeps the block dense.
A few small things worth flagging (none blocking):
- Lazy import reaches into
lib.board_hint._log_event— the leading underscore marks it as module-private by convention. Either promote to public (log_event) for cross-module callers, or wrap as a thin public helper. The current import works but technically violates the privacy hint. - Definition order:
_print_hintsis defined immediately after the imports block, aboveSHELL_COMMANDS/SPINNER_REand the other module-level constants. Existing convention in this file (and most of the cnb codebase) is constants-first, then helpers. Trivial move, but easier for the next reader. - Pagination via repeat-view: 10 pending hints take 2
cmd_viewcalls to fully surface (5 per call). I assume this is intended (limits per-view noise) — worth a one-line comment so the next reader doesn't try to "fix" it. - Race window: two concurrent
board viewcalls could both see the samestatus=pendingrow, both mark it surfaced, both emit asurfaceevent. Single-user is the dominant case and SQLite WAL is mostly serializing here, but worth noting if multi-user views ever happen. - Display ordering:
from {sender} ({ts}, conf {confidence:.2f}): {body}— the timestamp lands before the confidence. For skim-friendliness, I'd lead with confidence (the actionable signal) and puttslast as context. Pure cosmetic; the design doc shows the same(2h ago, conf 0.78)order so this matches the spec.
LGTM. With #249 and #250 already approved, the three-phase chain is ready end-to-end behind the [hints] enabled=false opt-in flag.
Summary
Phase 3 of #158 (design doc) — the surface UI. Eligible hints now appear as a yellow
💡 association hints:block at the top ofboard view, reusing PR #221's runtime-alert formatter. Stacks on #250 (phase 2). Completes the three-phase #158 chain.What it does
_print_hints(db, recipient)(inlib/board_view.py) runs after the unread-count alert incmd_view:hints WHERE recipient=? AND status='pending' AND confidence >= threshold ORDER BY confidence DESC, id DESC LIMIT 5.from <sender> (<ts>, conf <X.YY>): <body>in yellow, flipsstatus → surfaced, setssurfaced_at, and logs asurfaceevent tohint_events.[hints] enabled = trueinnotifications.toml(same flag as phase 1/2).The block borrows visual placement and weight from PR #221's alert block: top of view, ignorable, doesn't poison inbox.
Invariants
surfaced, the nextboard viewskips it (query isstatus='pending'only).LIMIT 5caps per-view noise even if rate-limits + threshold somehow let more accumulate.hint_events(alongside phase 1'semitevents) so v2 (learned model) has a complete trace.Test plan
pytest tests/test_hint_surface.py— 11/11 greenruff check+ruff formatcleanTest coverage:
pending → surfacedtransition,hint_eventsgainssurfacerow, surfaced hints don't re-surfacecmd_viewincludes the block when enabled, omits when disabledStack
cmd_sendhook)🤖 Generated with Claude Code