Conversation
…rint `dms --days 1` over an archive whose only direct message has no channels row printed "note: 1 messages in scope but none within the last 1 days; try without --days", and `dms` without the window then listed no conversation. With a catalogued conversation alongside, the note reported 2 messages and the newest timestamp of the row `dms` cannot print. store.DirectMessageConversations selects from channels and joins messages to it on both channel id and guild id, so a direct message whose channel was never catalogued under its guild is in that result at no window, while the count behind the note selected from messages and included it. MessageScopeOptions gains CataloguedChannelsOnly, the `dms` window note sets it, and the count is now taken over the listing's own row set: zero prints nothing, and above zero names rows that dropping the window returns. The correlated subquery is qualified on both sides and MessageScopeStats now qualifies its scope clauses with the messages table, because channels carries a guild_id column of its own and an unqualified outer reference inside the subquery resolves to the channels row. The store test pins that by adding a channels row with the orphan's id under a different guild: the count stays 1. Found by running every note's recommended command against a scratch archive. Claude-Session: https://claude.ai/code/session_019WpK9xb3Z2Zym1T9C7EGQH
Contributor
|
🦞👀 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
P2
Contributor
|
Codex review: needs maintainer review before merge. Reviewed September 15, 2026, 11:34 AM ET / 15:34 UTC. ClawSweeper reviewWhat this changesThe PR excludes uncatalogued direct messages from empty-result window notes, with regression tests and documentation explaining the count. Merge readiness✅ Ready for maintainer review This remains a useful, focused fix: current main and v0.15.1 still contain the diagnostic mismatch. No blocking defect was found, and the supplied before/after CLI transcript demonstrates the corrected behavior. Priority: P2 Review scores
Verification
How this fits togetherDiscrawl reads locally archived Discord messages from SQLite. Its DM command switches between conversation summaries and filtered messages; empty-result notes help users choose a useful follow-up command. flowchart TD
A[Local Discord archive] --> B[DM command]
B --> C[Filtered message query]
B --> D[Conversation listing]
C --> E[Empty result]
E --> F[Count catalogued DM messages]
F --> G[Window advice on stderr]
D --> H[Conversation summaries]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep diagnostic counts aligned with conversations the suggested command can display while preserving access to uncatalogued messages through existing message queries. Do we have a high-confidence way to reproduce the issue? Yes: an archive with an old catalogued DM and a newer uncatalogued DM exposes the mismatch through Is this the best way to solve the issue? Yes: the opt-in internal query restriction narrowly repairs the misleading advice without changing message retrieval or persistent storage. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against b1ea76ebbb40. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Problem This Solves
Fixes:
dms --days Nreports messages in scope thatdmswithout the window does not list, when a direct message's channel has nochannelsrow.User Impact
The window note now counts only conversations
dmscan return, so the command it recommends produces the rows the note named. On an archive whose only direct messages are uncatalogued, the note does not print.Why This Change Was Made
store.DirectMessageConversationsselects fromchannelsand joinsmessageson channel id and guild id, so a direct message whose channel was never catalogued under its guild is in nodmslisting at any window, while the count behind the note selects frommessagesand includes it.MessageScopeOptionsgainsCataloguedChannelsOnlyand thedmswindow note sets it, so the count and the listing read one row set. Both tables carry aguild_id, so the correlated subquery is qualified on both sides andMessageScopeStatsqualifies its scope clauses with the messages table: an unqualified outer reference inside the subquery resolves to thechannelsrow and reduces the clause to "a channels row with this id exists".Evidence
Scratch archive holding one catalogued conversation (newest 2020-03-01) and one direct message whose channel has no
channelsrow (2020-07-01). Before:After, running the command the note recommends, which returns the row it counted:
Three new tests. Two drive
Run()over an archive holding an uncatalogued direct message and fail without the scope change. The store test adds achannelsrow carrying the orphan's id under a different guild, and fails if the guild half of the subquery is dropped or its outer reference is left unqualified.make fmt,make lint, andmake test-coverage(85.9%) pass. I found this by running each note's recommended command against a scratch archive.https://claude.ai/code/session_019WpK9xb3Z2Zym1T9C7EGQH