devs/cproof
1
2
Fork 1
Code Issues 43 Pull Requests 6 Actions Packages Projects 1 Releases Wiki Activity

SQLite history load merges LMC corrections regardless of PREF_CORRECTION_ALLOW #125

New Issue
Open
opened 2026-05-16 14:52:02 +00:00 by jabber.developer2 · 0 comments
jabber.developer2 commented 2026-05-16 14:52:02 +00:00
Collaborator
Copy Link

Summary

When /correction off (i.e. PREF_CORRECTION_ALLOW = false) is set, the SQLite history backend still merges Last Message Correction (LMC) updates into the originals when re-loading chat history. The user sees the corrected text in the history pane instead of seeing the original and the correction as separate messages. This contradicts the documented semantics of /correction off introduced in #114 / PR #116, which state that "every correction (incoming, outgoing, carbon-reflected) is rendered as a fresh message instead of mutating an existing buffer entry".

Where the gap is

The merge happens at SQL level in _sqlite_get_previous_chat (src/database_sqlite.c:387) — the query unconditionally joins correction rows onto their originals and returns the corrected text via COALESCE:

SELECT COALESCE(B.`message`, A.`message`) AS message, ...
FROM `ChatLogs` AS A
LEFT JOIN `ChatLogs` AS B ON (A.`replaced_by_db_id` = B.`id` AND A.`from_jid` = B.`from_jid`)
WHERE (A.`replaces_db_id` IS NULL) ...

No matter what PREF_CORRECTION_ALLOW is, the loaded message is already the corrected text and the correction row itself is filtered out. The live (win_print_*) paths in src/ui/window.c honour PREF_CORRECTION_ALLOW for in-session corrections, but that gate does not extend to history fetched from DB.

The flatfile backend already handles this correctly — when PREF_CORRECTION_ALLOW is false, _flatfile_get_previous_chat (src/database_flatfile.c:875-888) skips _ff_apply_lmc and returns all parsed lines (originals + corrections) as separate ProfMessages. SQLite needs to do the analogous thing.

Reproduction

  1. With sqlite backend active, exchange messages where a peer sends an LMC correction (XEP-0308) — or use /correct to correct your own outgoing message
  2. Verify the correction is applied in-buffer (default /correction on behaviour)
  3. /correction off
  4. Close and re-open the chat window to force history re-load from DB
  5. Observe: the original message in history shows the corrected text, the correction row is invisible — same as with /correction on

Expected: with /correction off, the loaded history should show both the original message and the correction as separate entries, matching the flatfile behaviour and the in-session live path.

Suggested fix

Branch the SQL in _sqlite_get_previous_chat on PREF_CORRECTION_ALLOW:

  • LMC allowed: keep current query (LEFT JOIN + COALESCE + replaces_db_id IS NULL filter)
  • LMC disabled: drop the JOIN/COALESCE, drop the replaces_db_id filter, return raw rows

A working implementation already exists — it was developed in the original review iteration of PR #116 and reverted on request to keep that PR's scope tight (see the discussion under #116). Can be lifted from the diff history or re-derived.

Why this is split out from PR #116

The reviewer's comment on PR #116 ("Doesn't affect messages fetched from DB though") flagged the gap on the gated line in win_print_outgoing, but the fix lives in database_sqlite.c which the PR otherwise does not touch. To keep PR #116 focused on the in-session/UI half of /correction off and on the wider /history & /logging cleanup, the DB-side fix is being moved to a follow-up.

Acceptance

  • With sqlite backend and /correction off, history re-load shows original + correction as separate messages
  • With sqlite backend and /correction on (default), no behaviour change vs. today
  • Existing flatfile behaviour unchanged
  • No new sqlite warnings/errors in logs

References

  • PR #116 (introduces the live-path gate and documents the intended semantics)
  • Issue #114 (parent nodb backlog)
  • src/database_flatfile.c:875-888 — reference implementation in the flatfile backend
## Summary When `/correction off` (i.e. `PREF_CORRECTION_ALLOW = false`) is set, the SQLite history backend still merges Last Message Correction (LMC) updates into the originals when re-loading chat history. The user sees the corrected text in the history pane instead of seeing the original and the correction as separate messages. This contradicts the documented semantics of `/correction off` introduced in #114 / PR #116, which state that "every correction (incoming, outgoing, carbon-reflected) is rendered as a fresh message instead of mutating an existing buffer entry". ## Where the gap is The merge happens at SQL level in `_sqlite_get_previous_chat` ([src/database_sqlite.c:387](src/database_sqlite.c#L387)) — the query unconditionally joins correction rows onto their originals and returns the corrected text via `COALESCE`: ```sql SELECT COALESCE(B.`message`, A.`message`) AS message, ... FROM `ChatLogs` AS A LEFT JOIN `ChatLogs` AS B ON (A.`replaced_by_db_id` = B.`id` AND A.`from_jid` = B.`from_jid`) WHERE (A.`replaces_db_id` IS NULL) ... ``` No matter what `PREF_CORRECTION_ALLOW` is, the loaded message is already the corrected text and the correction row itself is filtered out. The live (`win_print_*`) paths in [src/ui/window.c](src/ui/window.c) honour `PREF_CORRECTION_ALLOW` for in-session corrections, but that gate does not extend to history fetched from DB. The flatfile backend already handles this correctly — when `PREF_CORRECTION_ALLOW` is `false`, `_flatfile_get_previous_chat` ([src/database_flatfile.c:875-888](src/database_flatfile.c#L875-L888)) skips `_ff_apply_lmc` and returns all parsed lines (originals + corrections) as separate `ProfMessage`s. SQLite needs to do the analogous thing. ## Reproduction 1. With sqlite backend active, exchange messages where a peer sends an LMC correction (XEP-0308) — or use `/correct` to correct your own outgoing message 2. Verify the correction is applied in-buffer (default `/correction on` behaviour) 3. `/correction off` 4. Close and re-open the chat window to force history re-load from DB 5. Observe: the original message in history shows the corrected text, the correction row is invisible — same as with `/correction on` Expected: with `/correction off`, the loaded history should show both the original message and the correction as separate entries, matching the flatfile behaviour and the in-session live path. ## Suggested fix Branch the SQL in `_sqlite_get_previous_chat` on `PREF_CORRECTION_ALLOW`: - LMC allowed: keep current query (LEFT JOIN + `COALESCE` + `replaces_db_id IS NULL` filter) - LMC disabled: drop the JOIN/COALESCE, drop the `replaces_db_id` filter, return raw rows A working implementation already exists — it was developed in the original review iteration of PR #116 and reverted on request to keep that PR's scope tight (see the discussion under #116). Can be lifted from the diff history or re-derived. ## Why this is split out from PR #116 The reviewer's comment on PR #116 ("Doesn't affect messages fetched from DB though") flagged the gap on the gated line in `win_print_outgoing`, but the fix lives in `database_sqlite.c` which the PR otherwise does not touch. To keep PR #116 focused on the in-session/UI half of `/correction off` and on the wider /history & /logging cleanup, the DB-side fix is being moved to a follow-up. ## Acceptance - [ ] With sqlite backend and `/correction off`, history re-load shows original + correction as separate messages - [ ] With sqlite backend and `/correction on` (default), no behaviour change vs. today - [ ] Existing flatfile behaviour unchanged - [ ] No new sqlite warnings/errors in logs ## References - PR #116 (introduces the live-path gate and documents the intended semantics) - Issue #114 (parent nodb backlog) - [src/database_flatfile.c:875-888](src/database_flatfile.c#L875-L888) — reference implementation in the flatfile backend
jabber.developer added this to the Plans (2025-2026) project 2026-05-18 09:46:12 +00:00
jabber.developer added the
Kind/Bug
Priority
Medium
3
Reviewed
Confirmed
1
 labels 2026-06-24 07:30:19 +00:00
jabber.developer moved this to To Do in Plans (2025-2026) on 2026-06-24 07:34:48 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Compat/Breaking
Breaking change that won't be backward compatible
Kind/Bug
Something is not working
Kind/Documentation
Documentation changes
Kind/Enhancement
Improve existing functionality
Kind/Feature
New functionality
Kind/Security
This is security issue
Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No Label
Kind/Bug
Priority
Medium
3
Reviewed
Confirmed
1
Milestone
No items
No Milestone
Projects
Clear projects
No project Plans (2025-2026)
Assignees
Clear assignees
admin jabber.developer jabber.developer2
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: devs/cproof#125
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?