The cleanup preview marked itself stale before it could be drawn. The
gate bumps a revision on release so a preview cannot survive a wipe; I
then made the preview take the gate, so its own release invalidated it
every single time and the apply button never appeared. The feature has
been shipping non-functional since it was written, with a self-test that
asserted the exact bump that killed it.
Read-only operations no longer move the revision, and preview and
maintenance have their own marks instead of borrowing Cleanup -- which
also stops the five-second metadata refresh from expiring previews, and
stops the UI announcing "another operation is running: cleanup" during a
VACUUM.
The JSON export produced invalid JSON. The chat relation kinds were
interpolated straight into the output, and interpolating an enum writes
its member name, so every message with a recognised relation came out as
"source_kind":LocalPlayer. That is the file a GDPR access request goes
out on. The self-test wrote a JSON file and never parsed it; it does now.
Retention with the limit at zero still deleted. The slider is labelled
"0 = never" and the sweep seeded 31 spec defaults unconditionally before
reading the user's overrides, so zero still lost free company, linkshell
and party history after ninety days -- and the short-circuit written for
exactly this case could never be reached, because the map was never
empty.
A wipe that worked reported that it had failed. VACUUM needs the
database to itself, the refilter walks a lazy reader on the primary
connection outside the lock, and the two collide -- after the DELETE has
committed. The delete paths no longer let that escape: the rows are
gone, an uncompacted file is a housekeeping problem, and telling
somebody their history is still there when it is not is a different kind
of problem.
Also:
- CSV cells starting with =, +, - or @ get a leading apostrophe. The
content is text other people typed into a chat channel and the file
exists to be opened in a spreadsheet.
- An export that matched nothing no longer replaces the previous one. It
used to write its header, move it into place, and then report that
nothing matched. Dalamud's save dialog offers no overwrite
confirmation to fall back on, so this is the part that had to move.
- The retention sweep says so when it loses the race for the gate, and
routes its notifications through the teardown check like everything
else.
The worst of them made the block's own privacy promise backwards.
PrivacyPersistChannels was given a non-empty field initializer so a
fresh config would record conversations only. Dalamud deserialises with
Json.NET's defaults, which means ObjectCreationHandling.Auto: a
collection field that already holds items gets *populated*, not
replaced. Verified against Newtonsoft 13.0.3 -- saved [] loads as the
initializer, saved [Say] loads as initializer plus Say. So the change
would have unioned the privacy-first list into every existing config on
load and switched channels back on that the user had unticked, while
also making the v24 migration unreachable and its self-test vacuous. The
field is empty again and the seeding moved to CreateFresh, which only
runs when there is no config file at all.
Cleanup could delete a channel it had promised to keep. The allowlist
could only name channels that were already in the database when the
preview ran, so an unrecognised channel whose first message arrived
afterwards fell outside it. Where the failsafe is on, the deletion now
names what goes -- known channels that are not on the list -- instead of
what stays. The window closes completely, and a listed channel that
happens to be empty right now is safe for the same reason.
The cleanup preview was the one long operation that never took the
shared lock, while holding an open reader across a full-table scan.
That is precisely the case the lock was written for.
Clearing the history reported success when it failed. ClearMessages
purges the full-text index between the delete and the VACUUM; if that
step throws, the plaintext stays on disk and the user was told it was
gone. It has its own error string now, in all 25 languages.
Also:
- One busy state for the whole tab. Cleanup, clear, maintenance and
export reach the same store, and per-section flags left two
destructive buttons live at once. The lock turned that into a refusal
rather than damage, but a refusal you have to trigger to discover is
not an answer.
- The gate carries a revision, bumped by every mutating operation that
finishes. A preview taken before a retention sweep no longer passes as
current afterwards: comparing it against the settings alone cannot see
that the rows it counted are gone.
- Database metadata moved to a worker. Checking "is anything busy" first
is not enough, because an operation can take the lock in the gap
before COUNT(*) runs, and then the game stands still for a whole file
rewrite.
- The clear hint stays hidden until the count has actually been read.
"0 messages are stored" in front of the clear button is a lie told at
the worst possible moment.
- Refusal notices read the operation once. Guard and name were two reads
of the same field, so a run finishing in between printed a sentence
that stopped at the colon.
- The retention sweep cannot start twice. The gate only goes busy once
the worker reaches TryBegin, and the due-check runs every tick.
- Teardown waits up to five seconds for the store to come free rather
than disposing the connection under a running VACUUM.
- Maintenance has its own flag and says so when it is refused; reload
gets the same guard as its neighbour; the breakdown tree keeps its
open state across a language switch.
The privacy filter only decides what gets written from now on. Whatever
was stored before the user narrowed their channels stays there until
something removes it, and that something has had no button since May.
Two rules shape the section, both because this deletes history and
cannot be undone:
- Without a preview the apply button does not exist. Not greyed out,
absent. A disabled button is something a user waits for; a missing one
is something they have to go and earn.
- A preview that no longer matches the settings counts as no preview.
The old version only recoloured the number and left the button live,
so a changed whitelist could be applied against counts computed for
the previous one.
The mapping from the live rule to CleanupRetainOnly is the part worth
reading twice. CleanupRetainOnly takes one set and deletes everything
else, so it can only stand in for the live rule where that rule narrows
something: filter off means nothing is filtered, and an empty list means
a full wipe, which has its own button and its own confirmation. Both
cases now say so instead of offering a destructive action that does not
mean what it looks like.
Inside that, the allowlist is the whitelist itself plus any stored
channel this build does not recognise while the unknown-channel failsafe
is on. Deriving it from the counts instead would delete messages that
arrive on a whitelisted but currently empty channel between the preview
and the apply, and dropping the unrecognised ones would defeat the
failsafe, which exists to hold on to a new patch's channel until the
user has decided about it.
The preview runs on a worker over its own connection. It is a GROUP BY
across every stored row, the old version ran it inline on the draw
thread, and holding the read lock for it would stall UpsertMessage on
the framework thread for the length of the scan.
Cleanup_Help_SavedNote stays unused: it tells the reader to press Save
first, and the window it was written for had a Save button.
The exporter has worked since v1.4.8. The form that drives it went out
with the old settings window in May, which left PRIVACY.md promising an
access request the plugin had no way to answer.
New section in the data and privacy tab: time range, sender substring,
channel groups, format, and a save dialog. Form state lives in the tab,
not the config -- a filter describes one action, and a stale "last 7
days, sender Mira" reappearing weeks later is a worse start than an
empty form.
StreamForExport now takes a caller-owned connection. The reader stays
open for as long as the file is written, seconds to minutes on a large
history, and chat keeps arriving throughout -- so the primary connection
would be read here and written by UpsertMessage at once, and
SqliteConnection is not thread-safe. Holding the read lock instead would
trade that for freezing the game.
ChannelGroups lifts the eight groups out of the deleted tab and finishes
them: 37 of 89 channels belonged to no group and were therefore
unreachable in the UI. Game Master channels follow ChatTypeExt.Parent(),
so GmTell sits with the other tells rather than under system traffic --
an access request that quietly drops part of what it promises is the
dangerous kind of gap.
Also here:
- OpenSecondaryConnection disposes on a failing pragma. Open can succeed
and journal_mode=WAL still time out, and with Pooling=false the
connection then survives until a finalizer reaches it. Affects the
full-text rebuild worker too.
- StreamForExport builds its logger before the reader, so a throwing
CreateLogger cannot leave a reader nobody owns.
- The export thread takes the gate itself instead of the caller taking
it first. Acquiring before Start would strand the gate for the session
if thread creation failed, and the gate also holds back the sweep.
- Notifications are skipped once teardown has started. The thread has no
cancellation path and finishing the file is right, but reporting it to
a plugin that is gone is not.
- Transient widget rows that return their value instead of saving it.
Writing the config file on every keystroke of a sender filter would be
both pointless and slow.
- Five translated keys for "another database operation is running", in
all 25 languages. Two of the four operation names have no trigger yet;
they arrive with the cleanup and maintenance sections.
messages_fts stores sender and content as plain text, and no delete path touched
it. ClearMessages, CleanupRetainOnly and the retention sweep all removed rows
from `messages` alone, so the readable text of every "deleted" message stayed on
disk.
It was self-sealing. InitFtsReadyCache treats a non-empty index as ready, so
after a wipe the index stayed full, the flag stayed true, and the rebuild that
would have cleared it never ran again.
This is not hypothetical: the retention sweep runs unattended every 24 hours,
so any user with retention on has been accumulating orphaned plain text since
the index shipped. And the plugin says otherwise in two places -- the clear
button promises "Removes all message history. Cannot be restored", and
PRIVACY.md documents targeted deletion as a feature.
Wiping the index rather than deleting matched rows, because message_guid is a
GUID string while messages.Id is a BLOB and the two cannot be joined in SQL. The
index is derived data; it rebuilds from the surviving rows on the next start,
which is both cheaper and provably complete.
CleanupRetainOnly also skips VACUUM when nothing matched, the way
DeleteByRetentionPolicy already did. Rewriting the whole file for zero deleted
rows costs seconds on a large database and gains nothing.
Six tests drive the real store against a real database, since the defect was in
what the SQL did not touch rather than in any computed value.
Three of the four first-run profiles switch the retention sweep on and write a
per-channel deletion policy. The window only ever showed the global default, so
whatever the wizard decided about individual channels was invisible from the
moment the wizard closed.
The tab now lists every channel whose retention differs from the global value,
tagged by where the number comes from, plus a button to drop the custom ones. It
also finally uses the strings written for this screen, sweep description and
default help included.
Read-only, deliberately. Zero means "keep forever" as a global default and
"delete this channel's entire history" as a per-channel value, because the
default takes a separate SQL branch while a mapped channel gets cutoff = now. No
shipped profile contains a zero and no UI could set one, so nothing is broken
today -- but an editor cannot be offered until those two meanings agree. Noted
at the branch in DeleteByRetentionPolicy.
The default slider now goes down to 0, which is the value that means "never
delete anything". Its own label has been promising "0 = never" while the slider
started at 1.
GetTellHistoryWithSender filters on Receiver and orders by Date DESC. Without a
matching index SQLite sorted the whole receiver history into a temp b-tree
before yielding row one -- measured 10 ms to first row against 9621 tells, all
of it under TabsListLock, which defeats the early break in the caller.
(Receiver, ChatType, Date) does not help: the ChatType IN filter sits between
the equality prefix and the sort column, so the temp b-tree stays. Verified on
a real database: plan now reads SEARCH ... USING INDEX idx_messages_receiver_date
with no sort step.
No SQL LIMIT -- an earlier 500-row cap was removed in v1.4.10 because it cut
less-frequent partners off the back of the window, and 83% of partners have
fewer than 21 tells in total.
The migration dispatcher is cumulative, so Migrate5 is appended to every
existing case, not just the new one.
MessageStore, MessageEnumerator, MessageManager, AutoTellTabsService
move from Plugin.LogProxy / IPluginLogProxy onto
Microsoft.Extensions.Logging.ILogger<T> via constructor injection.
MessageStore additionally takes ILoggerFactory so it can build a
per-instance ILogger<MessageEnumerator> at each of the five reader-
spawning sites; the enumerator is not a container singleton.
PluginHostFactory's MessageManager and AutoTellTabsService factory
lambdas grow to resolve the new logger args; everything else stays in
place.
Site-level migration in the four files:
- MessageStore: 12 calls, _logger field IPluginLogProxy -> ILogger<MessageStore>
- MessageManager: 7 Plugin.LogProxy.* sites, new _logger field
- AutoTellTabsService: 9 Plugin.LogProxy.* sites, new _logger field
Plus a pre-existing template bug surfaced by CA2017: a LogDebug call
in AutoTellTabsService used "{tab.Name}" with no `$` prefix, which
landed in xllog as literal text under Plugin.LogProxy; ILogger now
reads that as a structured placeholder, so the call was promoted to
proper structured logging with tab.Name passed as a parameter.
Five trim spots from the cycle's earlier commits — none change behaviour,
just drop redundant phrasing and stale references per the HellionChat
comment-style convention (1-3 lines default, link "same as X" instead of
repeating, file:line refs only where they aid navigation).
SymbolPicker:
- BmpWhitelist header consolidated to source + filter ranges
- ImRaii.Popup pattern links the established ChatLogWindow popup idiom
instead of citing three call-sites
- ToIconString comment drops the "discoverability" footnote that the
code already telegraphs
- Manually-wrapping comment drops the "same modern idiom" tail that
duplicated the preceding sentence
MessageStore:
- Merge the stale pre-v1.4.10 sqlScanLimit comment with the new
v1.4.10 commentary; the cap mention now describes the historical
reason rather than a parameter that no longer exists
PreloadHistory had a hardcoded 500-row SQL scan window that capped the
per-partner history pull regardless of the AutoTellTabsHistoryPreload
setting. For active users with many tell partners, the scan window
filled up with chatter from other partners and pushed less-frequent
partners' history off the back end — pinned tabs reloaded empty even
though the messages were still in the database.
Drops the hardcoded scan cap. The (Receiver, Date) index keeps SQL fast
on the now-unbounded read, and the client-side loop still breaks as
soon as the configured per-tab limit is hit, so decode cost stays
proportional to the depth at which `limit` matches accumulate (typically
shallow even for chatty users).
Bump AutoTranslate-warmup and FilterAllTabs log-level from Debug to
Information so the xllog tail surfaces them without a Debug filter.
Wrap MessageStore.Connect and MessageStore.Migrate in Stopwatches so
the SQLite open and migration-chain costs are visible too.
Sub-Task 3.4 Befund on v1.4.8-baseline (4 reloads, medians):
- MessageStore.Connect: 50.5 ms
- MessageStore.Migrate: 2 ms
- MessageManager.FilterAllTabs: 68.5 ms
- AutoTranslate warmup: 108 ms
- UiBuilder HITCH: 108.9 ms
Outcome D — none of the three dominates the 200 ms threshold. The
ChatTwo "300 ms" comment for AutoTranslate is falsified at ~108 ms;
SQLite is not the bottleneck (52.5 ms total); FilterAllTabs runs on
the worker thread and only competes for CPU slots. The HITCH is left
unexplained by these probes, which keeps Hypothesis c (multi-window
WindowSystem.Draw initial pass) as the main R2 suspect to be
validated by the R1 lazy-window refactor.
Logs stay in as belt-and-suspenders for future plugin-load
regressions.
messages.Id is declared BLOB but stored as TEXT because Microsoft.Data.Sqlite
binds Guid parameters as UUID strings (UpsertMessage uses AddWithValue with
a Guid). RebuildFtsIndex cast reader.GetValue(0) to byte[] and threw
InvalidCastException at the first row. LoadByGuids bound byte[] params
against the TEXT-stored Id and would have returned no rows once the index
had built.
- RebuildFtsIndex reads via GetGuid and stores ToString() in
messages_fts.message_guid.
- LoadByGuids parses incoming UUID strings and binds them as Guid so
Microsoft.Data.Sqlite re-serialises to TEXT, matching the messages.Id
storage form.
- DbViewer caller variable renamed hexIds -> guidHits for clarity.
Two new public query methods plus an internal EscapeFtsTerm helper:
- FullTextSearch(term, limit) runs MATCH against messages_fts and returns
hex-encoded GUIDs sorted by FTS5 rank. Empty/whitespace short-circuits
to an empty list so callers can fall back to the local page filter.
- LoadByGuids(hexIds) resolves the hex GUIDs back to Message rows via
WHERE Id IN (...). Chunked at 500 to stay below SQLite's 999-parameter
cap, and the BLOB-PK autoindex means the join is O(log n) per id.
- EscapeFtsTerm wraps user input in double-quotes so multi-word queries
match as a phrase, not as per-word AND. Users opt into raw MATCH
syntax by writing their own quotes.
Plus _readLock serialises every Connection-touching internal method
(UpsertMessage, MessageCount, all readers, retention writers, etc.).
The DbViewer filter worker now runs FullTextSearch on a Task.Run thread
while the PendingMessageThread keeps calling UpsertMessage; SqliteConnection
is not safe for concurrent use, so this single lock is the minimal
architecture change that closes the race. The Lazy-Enumerator methods
(StreamForExport, GetDateRange, GetPagedDateRange) hold the lock only
through command-setup + ExecuteReader; v1.4.8 doc-notes the caveat for
the v1.5.x DI cycle to address with a snapshot-to-list or connection pool.
RebuildFtsIndex stays outside the lock -- it owns its own SqliteConnection
via OpenSecondaryConnection.
Adds the worker that fills the messages_fts virtual table after Migrate4.
The bulk-insert runs off the framework thread on its own SqliteConnection
opened via OpenSecondaryConnection -- WAL lets the live UpsertMessage
path on the primary Connection keep flowing, and the worker's writer
lock yields every 500 rows with a 5ms breather so PendingMessageThread
does not hit "database is locked" after DefaultTimeout=5s.
InitFtsReadyCache runs in the ctor and short-circuits to ready=true when
the index is already populated or when the messages table is empty. The
DbViewer (Task 4.4) reads IsFtsIndexBuilt per frame as a single volatile
field read.
Plugin.cs LoadAsync kicks the worker after FilterAllTabsAsync, gated on
IsFtsIndexBuilt and a CancellationTokenSource that DisposeAsync cancels
before MessageManager tears down. Progress reports back via IActiveNotification,
marshalled onto the framework thread via Framework.RunOnTick. Success path
finishes the notification as Success with a 5s linger; cancellation
dismisses it; an error swaps the type to Error with a fallback hint.
Pre-step for the v1.4.8 FTS5 bulk-insert worker. The worker opens its
own secondary SqliteConnection on the same db path so the WAL journal
lets parallel reads/writes through, and it has to apply the exact same
connection-string options and PRAGMAs as Connect() -- otherwise the
worker connection drifts the moment Connect grows a new pragma.
Splitting BuildConnectionString + ApplyPragmas out lets both Connect()
and the upcoming OpenSecondaryConnection() share the same source of
truth instead of duplicating the body. No behaviour change.
Lays down a messages_fts virtual table with message_guid (UNINDEXED, hex
TEXT of the BLOB primary key), sender_text and content_text columns
using the unicode61 tokenizer with diacritic folding. Standalone FTS5
without content='messages' linking, because messages.Id is BLOB and
FTS5's content_rowid contract requires an INTEGER rowid alias.
LoadByGuids (Task 4.3) will resolve the hex GUIDs back to messages rows
via WHERE Id IN (...) joins. Schema step only -- the bulk-insert worker
that fills the index lives in Task 4.2.
Internal Connection property exposure plus a HasMessagesFtsTable helper
let the Build-Suite verify Migrate4 without raw PRAGMA glue in each test.
v1.4.8 H2 Sub-Task 4.1.
Pure deserialisation helper that pulls one row from the current reader
position into a Message. The MessageEnumerator load path delegates to
it, and the upcoming FTS-join LoadByGuids (Task 4.3) will share the
same code so both stay in lockstep when the column layout shifts.
Pre-step for v1.4.8 H2 FTS5 full-text search.
MessageStore's Migrate0 (and the Migrate1/2/3 siblings) called
Plugin.Log.Information directly, which prevented an isolated xUnit
construction test from running — Dalamud.dll cannot load in the test
AppDomain. With IPluginLogProxy threaded through the ctor and the inner
MessageEnumerator, the whole MessageStore.cs file is now Dalamud-static
free and the Build-Suite covers it (Floor 688 -> 690).
This is the second half of F12.2; the remaining ~82 Plugin.Log call
sites in the rest of the plugin will be routed through the static
Plugin.LogProxy wrapper in a follow-up commit.
MessageStore.Connect used to call Util.IsWine() directly via a
DalamudUtil alias, which made the ctor unreachable from the xUnit
test AppDomain: any test that allocated a MessageStore tripped a
FileNotFoundException on Dalamud.dll before reaching the assertion.
The ctor now takes an IPlatformUtil and reads the cached IsWine
property. MessageManager passes Plugin.PlatformUtil in. Production
behaviour is identical; the test path can now substitute a fake
and exercise the SQLite migration logic in isolation.
- Translated project documentation (LEARNING-JOURNEY, CONTRIBUTORS, AI_DISCLOSURE) to English for better accessibility.
- Standardized internal code documentation by converting XML-doc blocks to standard comment format.
- Cleaned up inline comments and removed redundant versioning metadata across the codebase.
- Refactored non-functional text elements to improve readability and maintain a consistent style.
Add .prettierrc.json, .markdownlint.json, .yamllint.yaml, .gitattributes
Run CSharpier, Prettier and markdownlint across the entire codebase.
No logic changes — formatting, using order and line endings only.
Match the new HellionChat comment-length convention: 1-3 lines for
standard pitfall notes, 5+ only for non-trivial workarounds. The
previous Dispose comment was 14 lines of textbook prose, which veered
into AI-slop territory and would rot on the next refactor.
PendingSync läuft jetzt als LinkedList (O(1) Last statt O(n) Linq-Last
im ContentIdResolverHook); Privacy-Filter-Drop-Log auf Verbose runter,
sodass der Default-xllog-Stream nicht mehr pro Nachricht spammt.
JonKazama-Hellion