From d7957d5a3d3c75e53dfd70e8e7cbcd83a5bb2114 Mon Sep 17 00:00:00 2001 From: Robert Navarro Date: Mon, 14 Sep 2026 22:20:15 -0700 Subject: [PATCH] fix(cli): count dms window notes over conversations the listing can print `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 --- docs/commands/dms.md | 2 + internal/cli/zero_result_notes.go | 25 ++++--- internal/cli/zero_result_notes_test.go | 93 ++++++++++++++++++++++++++ internal/store/query.go | 32 +++++++-- internal/store/store_test.go | 68 +++++++++++++++++++ 5 files changed, 207 insertions(+), 13 deletions(-) diff --git a/docs/commands/dms.md b/docs/commands/dms.md index e57b23a..fc53c55 100644 --- a/docs/commands/dms.md +++ b/docs/commands/dms.md @@ -37,6 +37,8 @@ discrawl dms --with Molty --search "invoice" `dms` reads the same tables as [`messages`](messages.html) and [`search`](search.html) and prints the same stderr notes when a run returns nothing: the window notes for a `--hours`/`--days`/`--since`/`--before` listing, and the multi-term note for `--search`. They go to stderr, `--json` suppresses them, and stdout and the exit code are unchanged. +The window notes count only messages in a conversation `dms` can list. A direct message whose channel has no `channels` row is returned by no `dms` listing at any window, so it is left out of the count rather than reported as a row that dropping the window would show. + A run that sets `--with` gets no note. `--with` names a person and the query matches it against channel id and channel name alike, which the counts behind a note cannot reproduce, so a `--with` run stays silent rather than reporting numbers taken from every conversation. ## See also diff --git a/internal/cli/zero_result_notes.go b/internal/cli/zero_result_notes.go index acbfda1..507e27f 100644 --- a/internal/cli/zero_result_notes.go +++ b/internal/cli/zero_result_notes.go @@ -18,10 +18,11 @@ const maxZeroResultTermProbes = 4 // run with, so every note below is derived from the same row set the query // looked at rather than a re-modelled one. type zeroResultScope struct { - channelID string - guildIDs []string - includeEmpty bool - includeDeleted bool + channelID string + guildIDs []string + includeEmpty bool + includeDeleted bool + cataloguedChannelsOnly bool } // listMessagesScope models a store.ListMessages query. ListMessages carries no @@ -60,10 +61,11 @@ func (r *runtime) newZeroResultScope(channel string, guildIDs []string, includeE func (s zeroResultScope) storeOptions() store.MessageScopeOptions { return store.MessageScopeOptions{ - ChannelID: s.channelID, - GuildIDs: s.guildIDs, - IncludeEmpty: s.includeEmpty, - IncludeDeleted: s.includeDeleted, + ChannelID: s.channelID, + GuildIDs: s.guildIDs, + IncludeEmpty: s.includeEmpty, + IncludeDeleted: s.includeDeleted, + CataloguedChannelsOnly: s.cataloguedChannelsOnly, } } @@ -151,6 +153,12 @@ func (r *runtime) explainEmptySearch(opts store.SearchOptions, mode string) { // explainEmptyDirectMessageList explains an empty `dms` listing. Only the // window notes can apply: `dms` has no --channel, so there is no concrete // channel for the content notes to describe. +// +// The scope is narrowed to catalogued conversations. Dropping the window sends +// the reader to store.DirectMessageConversations, which selects from channels +// and joins messages to it, so a direct message whose channel has no channels +// row is absent from that listing at every window. Counting it would name rows +// the recommended command does not return. func (r *runtime) explainEmptyDirectMessageList(with string, includeEmpty bool, window zeroResultWindow) { if r.json || strings.TrimSpace(with) != "" { return @@ -159,6 +167,7 @@ func (r *runtime) explainEmptyDirectMessageList(with string, includeEmpty bool, if !ok { return } + scope.cataloguedChannelsOnly = true r.explainEmptyMessages(scope, window) } diff --git a/internal/cli/zero_result_notes_test.go b/internal/cli/zero_result_notes_test.go index c3137c9..e69413b 100644 --- a/internal/cli/zero_result_notes_test.go +++ b/internal/cli/zero_result_notes_test.go @@ -26,6 +26,7 @@ const ( zeroResultDeletedChannelID = "6666666666666666" zeroResultDMChannelID = "7777777777777777" zeroResultOrphanChannelID = "8888888888888888" + zeroResultOrphanDMChannelID = "9999999999999999" ) func setupZeroResultStore(t *testing.T) (ctx context.Context, cfgPath string) { @@ -1017,3 +1018,95 @@ func TestExplainEmptyResultsUncataloguedDMDoesNotRecommendEmbedding(t *testing.T require.Contains(t, stderr.String(), "embedding jobs are never created for direct messages") require.NotContains(t, stderr.String(), "--rebuild") } + +// addUncataloguedDirectMessage stores a direct message whose channel has no +// channels row, which is what an archive holds after a DM is captured without +// its conversation ever being catalogued. +func addUncataloguedDirectMessage(t *testing.T, ctx context.Context, cfgPath string) { + t.Helper() + cfg, err := config.Load(cfgPath) + require.NoError(t, err) + s, err := store.Open(ctx, cfg.DBPath) + require.NoError(t, err) + require.NoError(t, s.UpsertGuild(ctx, store.GuildRecord{ID: store.DirectMessageGuildID, Name: "Direct Messages", RawJSON: `{}`})) + require.NoError(t, s.UpsertMessage(ctx, store.MessageRecord{ + ID: "m-dm-uncatalogued", + GuildID: store.DirectMessageGuildID, + ChannelID: zeroResultOrphanDMChannelID, + ChannelName: "Bob", + AuthorID: "u3", + AuthorName: "Bob", + CreatedAt: "2020-07-01T00:00:00Z", + Content: "quebec has no channels row", + NormalizedContent: "quebec has no channels row", + RawJSON: `{}`, + })) + require.NoError(t, s.Close()) +} + +// `dms --days 1` over an archive holding one catalogued conversation and one +// direct message with no channels row reported 2 messages in scope and the +// newest timestamp of the row `dms` cannot print. +// store.DirectMessageConversations selects from channels and joins messages to +// it, so the uncatalogued message is in no listing at any window. The count +// behind the note now matches the listing's own row set. +func TestExplainEmptyResults_DirectMessageWindowNoteCountsOnlyCataloguedConversations(t *testing.T) { + ctx, cfgPath := setupZeroResultStore(t) + addUncataloguedDirectMessage(t, ctx, cfgPath) + + var stdout, stderr bytes.Buffer + require.NoError(t, Run(ctx, []string{"--config", cfgPath, "dms", "--days", "1"}, &stdout, &stderr)) + require.Empty(t, stdout.String()) + // One conversation, and the newest timestamp is that conversation's, not + // the uncatalogued message's 2020-07-01. + require.Contains(t, stderr.String(), "note: 1 messages in scope but none within the last 1 days (newest: 2020-03-01T00:00:00Z); try without --days") + + // The command the note recommends lists the conversation it counted. + stdout.Reset() + stderr.Reset() + require.NoError(t, Run(ctx, []string{"--config", cfgPath, "dms"}, &stdout, &stderr)) + require.Contains(t, stdout.String(), zeroResultDMChannelID) + require.NotContains(t, stdout.String(), zeroResultOrphanDMChannelID) + require.Empty(t, stderr.String()) +} + +// The same fix from the other side: when every direct message in the archive +// has no channels row, `dms` returns no conversation at any window, so +// dropping the window resolves nothing and there is no note to print. +func TestExplainEmptyResults_DirectMessageWindowNoteSilentWhenNoConversationIsListed(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.toml") + cfg := config.Default() + cfg.DBPath = filepath.Join(dir, "discrawl.db") + require.NoError(t, config.Write(cfgPath, cfg)) + addUncataloguedDirectMessage(t, ctx, cfgPath) + + var stdout, stderr bytes.Buffer + require.NoError(t, Run(ctx, []string{"--config", cfgPath, "dms", "--days", "1"}, &stdout, &stderr)) + require.Empty(t, stdout.String()) + require.Empty(t, stderr.String()) + + // Dropping the window, which is what a window note would have recommended, + // returns no conversation either. The human listing still prints its column + // header, as every table in this CLI does with no rows. + stdout.Reset() + stderr.Reset() + require.NoError(t, Run(ctx, []string{"--config", cfgPath, "--json", "dms"}, &stdout, &stderr)) + require.Equal(t, "[]\n", stdout.String()) + require.Empty(t, stderr.String()) + + stdout.Reset() + stderr.Reset() + require.NoError(t, Run(ctx, []string{"--config", cfgPath, "dms"}, &stdout, &stderr)) + require.NotContains(t, stdout.String(), zeroResultOrphanDMChannelID) + + // The message is reachable, just not as a conversation, so the archive is + // not simply empty. + stdout.Reset() + stderr.Reset() + require.NoError(t, Run(ctx, []string{ + "--config", cfgPath, "messages", "--channel", zeroResultOrphanDMChannelID, + }, &stdout, &stderr)) + require.Contains(t, stdout.String(), "quebec has no channels row") +} diff --git a/internal/store/query.go b/internal/store/query.go index a76e498..0529204 100644 --- a/internal/store/query.go +++ b/internal/store/query.go @@ -87,11 +87,20 @@ func (s *Store) ChannelMessageBounds(ctx context.Context, channelID string) (str // SearchMessages filters `deleted_at is null`, ListMessages carries no // deleted_at predicate and so returns them. A caller sets it to match the // query it is explaining. +// +// CataloguedChannelsOnly exists for the same reason. Queries that select from +// messages reach a channel with no channels row, and +// DirectMessageConversations selects from channels and joins messages to it on +// both channel id and guild id, so a message whose channel was never +// catalogued under its guild is in that result at no window. A caller +// explaining that listing sets this; a caller explaining a messages-driven +// query does not. type MessageScopeOptions struct { - ChannelID string - GuildIDs []string - IncludeEmpty bool - IncludeDeleted bool + ChannelID string + GuildIDs []string + IncludeEmpty bool + IncludeDeleted bool + CataloguedChannelsOnly bool } // MessageScopeStats summarises the messages allowed by the scope. Count, @@ -123,11 +132,24 @@ func messageScopeClauses(opts MessageScopeOptions, column func(string) string) ( args = append(args, guildID) } } + if opts.CataloguedChannelsOnly { + // Both tables carry channel-identifying columns, so every reference + // here is qualified: the unqualified name inside the subquery would + // resolve to the channels row it selects from, which would compare + // that row against itself and quietly reduce the clause to "some + // channels row has this id". Callers pass a column function that + // qualifies with the outer messages table for the same reason. + clauses = append(clauses, "exists (select 1 from channels cat where cat.id = "+column("channel_id")+ + " and cat.guild_id = "+column("guild_id")+")") + } return strings.Join(clauses, " and "), args } func (s *Store) MessageScopeStats(ctx context.Context, opts MessageScopeOptions) (MessageScopeStats, error) { - where, whereArgs := messageScopeClauses(opts, func(name string) string { return name }) + // Qualified with the table this statement selects from, so a correlated + // subquery in the scope clauses binds to this query's messages row rather + // than to whatever row the subquery itself selects. + where, whereArgs := messageScopeClauses(opts, func(name string) string { return "messages." + name }) visible := "(? or trim(coalesce(normalized_content, '')) <> '')" args := []any{opts.IncludeEmpty, opts.IncludeEmpty, opts.IncludeEmpty, opts.IncludeEmpty} args = append(args, whereArgs...) diff --git a/internal/store/store_test.go b/internal/store/store_test.go index e699e3d..e1863af 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -2681,3 +2681,71 @@ func TestMessagePendingEmbeddingJobsCountsWhatEmbedWouldDrain(t *testing.T) { require.Equal(t, 1, stats.DirectMessages) require.Zero(t, stats.Embeddable) } + +// CataloguedChannelsOnly narrows a scope to the rows +// DirectMessageConversations can return. That listing selects from channels +// and joins messages on both channel id and guild id, so a direct message +// whose channel has no channels row under its guild is in no listing at any +// window, and a count taken over messages alone reports rows the listing +// cannot produce. +func TestMessageScopeStatsCataloguedChannelsOnlyMatchesTheConversationListing(t *testing.T) { + t.Parallel() + + ctx := context.Background() + s, err := Open(ctx, filepath.Join(t.TempDir(), "discrawl.db")) + require.NoError(t, err) + defer func() { _ = s.Close() }() + + require.NoError(t, s.UpsertGuild(ctx, GuildRecord{ID: DirectMessageGuildID, Name: "Direct Messages", RawJSON: `{}`})) + require.NoError(t, s.UpsertChannel(ctx, ChannelRecord{ + ID: "c-dm", GuildID: DirectMessageGuildID, Kind: "dm", Name: "Alice", RawJSON: `{}`, + })) + dm := func(id, channelID, created string) MessageRecord { + return MessageRecord{ + ID: id, GuildID: DirectMessageGuildID, ChannelID: channelID, ChannelName: "Alice", + AuthorID: "u2", CreatedAt: created, Content: "hello", NormalizedContent: "hello", RawJSON: `{}`, + } + } + require.NoError(t, s.UpsertMessage(ctx, dm("m-dm", "c-dm", "2026-01-02T00:00:00Z"))) + require.NoError(t, s.UpsertMessage(ctx, dm("m-dm-orphan", "c-dm-orphan", "2026-01-03T00:00:00Z"))) + + // Both messages are in the guild scope, and the newest of them is the one + // with no channels row. + stats, err := s.MessageScopeStats(ctx, MessageScopeOptions{GuildIDs: []string{DirectMessageGuildID}}) + require.NoError(t, err) + require.Equal(t, 2, stats.Count) + require.Equal(t, parseTime("2026-01-03T00:00:00Z"), stats.Newest) + + // The listing returns one conversation, and the narrowed scope counts one. + rows, err := s.DirectMessageConversations(ctx, DirectMessageConversationOptions{}) + require.NoError(t, err) + require.Len(t, rows, 1) + require.Equal(t, "c-dm", rows[0].ChannelID) + + stats, err = s.MessageScopeStats(ctx, MessageScopeOptions{ + GuildIDs: []string{DirectMessageGuildID}, CataloguedChannelsOnly: true, + }) + require.NoError(t, err) + require.Equal(t, 1, stats.Count) + require.Equal(t, parseTime("2026-01-02T00:00:00Z"), stats.Newest) + + // The listing joins on guild as well as channel id, so a channels row + // carrying the same id under a different guild leaves the orphan out. Both + // tables carry a guild_id, so this also pins the correlated subquery to the + // messages row: with the guild half dropped, or with its outer reference + // resolving to the channels row it selects from, the count below is 2 and + // the newest timestamp is 2026-01-03. + require.NoError(t, s.UpsertChannel(ctx, ChannelRecord{ + ID: "c-dm-orphan", GuildID: "g1", Kind: "text", Name: "same id, other guild", RawJSON: `{}`, + })) + rows, err = s.DirectMessageConversations(ctx, DirectMessageConversationOptions{}) + require.NoError(t, err) + require.Len(t, rows, 1) + + stats, err = s.MessageScopeStats(ctx, MessageScopeOptions{ + GuildIDs: []string{DirectMessageGuildID}, CataloguedChannelsOnly: true, + }) + require.NoError(t, err) + require.Equal(t, 1, stats.Count) + require.Equal(t, parseTime("2026-01-02T00:00:00Z"), stats.Newest) +}