fix(cli): explain empty search, messages, and dms results - #233
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P3
|
Codex review: blocked before merge. Reviewed September 13, 2026, 10:44 PM ET / September 14, 2026, 02:44 UTC (Revision 5). ClawSweeper reviewWhat this changesAdds explanations and recovery suggestions on stderr for empty local search, message, and DM results, with supporting queries, documentation, and regression tests. Merge readiness⛔ Blocked before merge - 1 item remains The contribution remains useful and is absent from current main and v0.15.0. The prior recovery and documentation findings are addressed, and no remaining blocking defect was found. Priority: P3 Review scores
Verification
How this fits togetherDiscrawl searches and lists Discord messages stored in a local SQLite archive. The new diagnostic layer examines empty query results and local archive coverage, then suggests recovery steps without changing stdout or query results. flowchart TD
A[CLI query and filters] --> B[Local archive query]
C[SQLite messages and embeddings] --> B
B --> D{Empty result?}
D -->|No| E[Existing stdout output]
D -->|Yes| F[Check scope and coverage]
C --> F
F --> G[Stderr advice unless JSON]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain scope-aware recovery advice, unchanged stdout and JSON behavior, and explicit documentation of the adopted stderr default. Do we have a high-confidence way to reproduce the issue? Not applicable as a remaining bug: this adds diagnostic behavior, and the previously identified defects are corrected in current source. Is this the best way to solve the issue? Yes. A shared, bounded diagnostic layer reuses existing query and recovery paths while preserving result semantics and suppressing explanations where the aggregate scope would be inaccurate. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against 072bf87a9974. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (4 earlier review cycles)
|
|
The date-window note said why an empty result was empty but not which channel it counted over: Every note that reports a count over a scope now names that scope through one helper, in the format the channel-empty note already used: "5 messages in channel 1494732128137515008 (Qwen3.6-35B-A3B, kind=thread_public) but none within the last 1 days (newest: 2026-04-17T23:31:41Z); try without --days". A guild-wide query keeps "in scope" and a resolved id that matches no Reason, timestamp and recommended command are unchanged, as are stderr-only delivery and |
|
Both P2 findings are fixed, in 0fa7f8d and 5f6c7a0.
The embedding note recommended On stderr by default, I am keeping it on and treating that as my decision rather than adding a flag. Anyone who folds stderr into stdout will see new lines after upgrading, and
|
search and messages returned zero rows, exit 0, and printed nothing in several situations where the data actually exists (never-crawled channel, a --days/--since window that misses everything, or an FTS query whose terms are implicitly ANDed together). Add stderr-only diagnostics for these cases without changing query semantics; they are suppressed under --json so stdout stays machine-parseable.
The notes explaining an empty search or messages result were derived from a row set the query never used, so several empty results stayed silent and one note recommended something this search path cannot do. - The multi-term note suggested an exact phrase. normalizeFTSQuery replaces double quotes with spaces and wraps each whitespace-delimited unit in an FTS5 phrase, so quoting changes nothing and the suggestion returns the same zero rows. Recommend a single distinctive term narrowed by --channel or --author, which this path does honour. - The same note was skipped whenever the query contained a double quote. Quoted and unquoted multi-term queries have identical semantics here, so both now get the note, and the reported term is rendered the way the index sees it. Terms come from store.FTSQueryTerms, the split normalizeFTSQuery itself uses. - A channel holding only empty or attachment-only messages produced no note: the stats query filtered on deleted_at while the message and search queries also drop rows with empty normalized content. MessageScopeStats now reports both counts and the notes name --include-empty for that case. - The --days/--since note needed a resolved --channel to fire. It now runs off the same guild scope the query used, so a guild-wide window explains itself. - --before had no note at all, and with --since set only --since was named. Each side of the window is evaluated and reported independently. - Resolving one channel id for the note text scanned every archived channel row. Added store.ChannelByID, a primary-key lookup. - The forum hint printed a SQL statement; it now names `channels list` and `messages --channel`. - A long query issued one extra probe search per word; probes are capped. - semantic and hybrid searches now explain a scope whose messages carry no embeddings for the configured provider and model, which a vector query cannot match, and hybrid also gets the FTS AND-hint for its lexical leg.
`messages --channel ID --days 1` printed "channel ID has no messages in the
local mirror" for a channel whose messages were deleted over the Gateway,
while plain `messages --channel ID` listed those same rows. store.ListMessages
carries no deleted_at predicate and returns soft-deleted rows; the stats query
behind the note always filtered them out, so the note described a narrower row
set than the query it was explaining. MessageScopeOptions now carries
IncludeDeleted and each entry point sets it to match its own query:
`messages` counts soft-deleted rows, `search` does not.
`dms` reaches the same store.ListMessages and store.SearchMessages queries and
returned zero rows just as silently, so it now carries the window and
multi-term notes. A `--with` run gets none: --with names a person and the query
matches it against channel id and channel name alike, which a channel_id-keyed
stats query cannot reproduce. --hours is a `dms` flag `messages` does not have
and now has its own note rather than falling through to the --days wording.
Documents the notes in docs/commands/{search,messages,dms}.md: they go to
stderr, --json suppresses them, and stdout and exit codes are unchanged.
Claude-Session: https://claude.ai/code/session_019WpK9xb3Z2Zym1T9C7EGQH
`messages --guild G --channel ID` printed "channel ID has no messages in the local mirror" when ID exists but belongs to a different guild, which the same command without --guild contradicts by printing rows. The note now names the channel's actual guild and the --guild value that reaches it. Covered through both `messages` and `search`, which share the helper that prints it. Covers --mode hybrid, whose lexical leg is the same FTS query: it takes the multi-term note, and the embedding-coverage note only once embeddings are configured. Claude-Session: https://claude.ai/code/session_019WpK9xb3Z2Zym1T9C7EGQH
`messages --channel ID --days 1` printed "5 messages in scope but none within the last 1 days (newest: ...)" where the channel-empty note on the same id printed "channel ID (Qwen3.6-35B-A3B, kind=thread_public)". The window note carries the accurate reason and dropped the identifying detail. Every note that reports a count over a scope now names that scope through one helper, in the format the channel-empty note already used: the single channel as "channel ID (name, kind=k)", a guild-wide query as "in scope", and a resolved id with no channels row as the bare id, so no note prints an empty parenthetical. That covers the --hours/--days/--since note, the --before note, the --include-empty note and the embedding-coverage note; the guild-scope note takes kind= from the same helper. Reason, timestamp and suggested command are unchanged, as are stderr-only delivery and --json suppression. Claude-Session: https://claude.ai/code/session_019WpK9xb3Z2Zym1T9C7EGQH
`messages --channel ID --hours 6` printed no explanation when every archived message in the channel predated the window. The initializer at the messages entry point supplied the resolved `since` timestamp but left `hours` zero, and `explainEmptyDateWindow` selects its wording from the flag fields, so none of its branches ran and the command stayed silent. Pass `hours` alongside the other three window flags. The new test drives `messages --channel ID --hours 6` through `Run()`, so the flag has to survive the entry point that was dropping it rather than being handed straight to the helper. Claude-Session: https://claude.ai/code/session_019WpK9xb3Z2Zym1T9C7EGQH
… search A semantic search over archived messages with no embedding job left the reader at `discrawl embed`, which drains jobs that already exist and creates none, so the run reported nothing to do and the next search was still empty. `runEmbed` creates missing jobs only under --rebuild, via store.RequeueAllEmbeddingJobs, and that requeues every non-deleted message outside DMs archive-wide rather than the queried scope. The note now reports which of three states the scope is in and names the command that applies to it. With pending jobs in scope, `discrawl embed` embeds them. With none, the note says `discrawl embed` drains nothing and names `discrawl embed --rebuild`, with its archive-wide scope and its --limit per run stated, so the scope is known before the command is run. A direct-message scope gets neither: InsertMissingEmbeddingJobs and the enqueue-on-write path both filter guild_id '@me', so no embed run gives those messages embeddings and the note recommends --mode fts alone. store.MessagePendingEmbeddingJobs supplies the count, applying the three predicates ListPendingEmbeddingJobs applies, so it counts what an embed run would act on rather than every embedding_jobs row. Each recommended command is executed in its own test through `Run()` against a stub OpenAI-compatible embedding server, and the search after it asserts rows come back. The fixture embeds the archive and then archives one more message, which is the only shape that reaches this note: store.SearchMessagesSemantic returns ErrNoCompatibleEmbeddings when the archive holds no compatible embeddings at all. Claude-Session: https://claude.ai/code/session_019WpK9xb3Z2Zym1T9C7EGQH
Preserve supported diagnostic scopes, explain combined date exclusions and archive-wide embedding limits, and isolate semantic fixtures from live credentials. Co-authored-by: Robert Navarro <crshman@gmail.com>
5f6c7a0 to
27d9f8a
Compare
steipete
changed the title
clawsweeper
Bot
added
rating: 🐚 platinum hermit
2 participants
Empty local
search,messages, anddmsresults now explain supported causes on stderr: uncrawled or forum channels, content filters, date windows, guild mismatches, multi-term search, and missing embedding coverage. JSON suppresses the notes; stdout and exit codes stay unchanged. The command documentation explains the stderr behavior and its limits. Aggregate notes are suppressed with--author, where message and search paths use different name matching; lexical term probes retain every filter. Contradictory date bounds get combined advice. Embedding notes respect local-only DMs, empty jobs, and the archive-wide queue limit.Rebased onto current main and fixed the three failing semantic tests: the local embedding stub now uses and verifies a synthetic test credential instead of inheriting OPENAI_API_KEY from the developer environment.
Validation: focused CLI/store tests with OPENAI_API_KEY unset, the complete local gate on the final candidate (86.0% coverage), documentation tests/build, independent P0–P2 review, and a built-CLI comparison against main on a synthetic archive. All 15 CLI cases preserve stdout and exit codes byte-for-byte, and the suggested follow-up commands return rows.
Thanks @rnavarro for the fix.
The revised full local gate passed; CI is running on the updated candidate. Review’s sole remaining report assumed date flags on search; the source has no such fields and the built CLI rejects them, so that report was rejected with evidence.