fix(ui): reset paged flag at buffer bottom for non-chat windows #117

Closed

jabber.developer2 wants to merge 1 commits from fix/paged-non-chat-windows into master

pull from: fix/paged-non-chat-windows

merge into: devs:master

devs:master

devs:feat/ai-api-type

devs:fix/editor-terminal-size

devs:ci/docker-hub-publishing

devs:feat/disco-ac

devs:feat/privacy-enhancements

devs:fix/clientid-regression

devs:fix/ai-chat-completions-followup

devs:feat/ai-custom

devs:fix/unencrypted-send

devs:rollback/pre-upstream-merge

devs:fix/autoping-warning-null-domain

devs:fix/pad-dead-space-reclaim

devs:feat/autoping-warning

devs:chore/untrack-gitversion

devs:fix/multiline-pad-clip

devs:fix/issue-112-followups

devs:fix/issue-128-migrate-v3-dedup

devs:fix/delay-timestamp-validation

devs:ref/light-cleanup

devs:fix/history-scroll-pad-redraw-storm

devs:fix/plugin-post-display-incoming-only

devs:merge/upstream-full

devs:merge-improve

devs:chore/remove-chatlog-stage-1

devs:fix/ai-json-encoding

devs:feat/no-db-backlog-114

devs:fix/scroll-non-chat-windows

devs:fix/ai-leaks

devs:feat/ai

devs:fix/ai-followups

devs:test/ai-coverage-unit-only

devs:test/ai-coverage

devs:refactor/scroll-mechanism

devs:fix/verify-per-contact-context

devs:feat/no-db-mode

devs:feat/ai-json

devs:feat/pikaur-parity-arch

devs:fix-pikaur-build

devs:feat/upstream-sync

devs:feat/functest-speedup

devs:fix/cwe-134-format-string-audit

devs:ci/separate-build-step

devs:test/autoping-functional-tests

devs:test/db-functional-tests

devs:fix/arch-build

devs:fix/xep-0030-disco-items-error-handling

devs:fix/xep-0030-empty-disco-items

devs:playground/fix/src_refactoring

devs:tests/disco

devs:fix/test-CI-stability

devs:feat/parallel-tests-clean

devs:feat/parallel-functional-tests

devs:fix/functional_tests_v2

devs:fix/functional_tests

devs:fix/connect_max_args

devs:feat/extended_debug_info

devs:playground/fix/scroll-stuck

devs:build/multicore

devs:build/reenable-fedora

devs:build/autoupdate

Conversation 0 Commits 1 Files Changed 1 +8

jabber.developer2 commented 2026-05-10 12:38:31 +00:00

Collaborator

Copy Link

Problem

When the user scrolls up in a non-WIN_CHAT window (AI chat, MUC, MUC PM, console, plugin, vcard, xml-console), win_page_up sets window->layout->paged = 1. Subsequent _win_printf calls then silently drop every new message:

// src/ui/window.c, _win_printf
if (window->layout->paged && wins_is_current(window)) {
    window->layout->unread_msg++;
    return;
}

The reset back to paged = 0 lives only inside the WIN_CHAT-gated DB-fetch block in win_page_down (lines 806–825): only WIN_CHAT can ever trigger WIN_SCROLL_REACHED_BOTTOM, which is the sole condition that flips paged off (line 847). For every other window type, paged stays 1 for the rest of the session — even after the user pages all the way back down to the bottom.

This is the bug behind the "AI window stops showing user messages after scrolling up" report, and the same mechanism breaks new-message rendering in MUC and console after a single page-up.

The existing cl_ev_send_ai_msg workaround on feat/ai (commit 010b2062a) only patches the user's own outgoing AI message: it resets paged right before win_print_outgoing. Incoming AI streaming responses bypass _win_printf (they go through win_println directly), which is why those still show — but the paged mechanic itself remains broken for everything else.

Fix

Add a generic bottom-reached reset at the end of win_page_down for non-chat window types:

// Non-chat windows have no DB to fetch from, so WIN_SCROLL_REACHED_BOTTOM
// never fires; reset paged once the buffer's last line is on screen.
if (window->type != WIN_CHAT
    && (total_rows - *page_start) <= page_space + 1) {
    window->layout->paged = 0;
    window->layout->unread_msg = 0;
}

The + 1 slack covers the past-end clamp branch a few lines above (*page_start = total_rows - page_space - 1), which lands total_rows - *page_start on page_space + 1 even though the user is visually at the bottom.

• It plugs the existing dispatch hole — paged was only reset on WIN_CHAT, this extends it to every other window type that uses _win_printf
• Symmetric to the existing WIN_SCROLL_REACHED_BOTTOM reset two lines up; same semantics, just unconditional on window type
• Fixes AI / MUC / MUC PM / console / vcard / plugin / xml-console all at once, not just AI
• Makes the targeted reset in cl_ev_send_ai_msg (010b2062a) redundant — that commit can be reverted as cleanup once this lands

Out of scope

• Suppressed messages (those _win_printf dropped while paged = 1) are still lost: _win_printf only increments unread_msg, it does not buffer the dropped content. This is a pre-existing flaw shared with the WIN_SCROLL_REACHED_BOTTOM path and is not introduced by this change. Recovering them is a separate task (either keep the message in the buffer + render later, or display below a "new messages" marker).
• The conceptual conflation of "buffer bottom" vs "DB bottom" in win_scroll_state_t for WIN_CHAT is not touched.

Test plan

• AI chat: send several messages → page up → page down → send a new message → message displays without needing the existing cl_ev_send_ai_msg reset
• MUC: receive enough messages to scroll → page up → page down → peer's next message displays
• MUC PM: same scenario as MUC
• Console: scroll up through cons output → page down → new console output displays
• Regular WIN_CHAT: page-up + page-down behaviour unchanged (still uses DB-fetch path; no double reset)

## Problem When the user scrolls up in a non-`WIN_CHAT` window (AI chat, MUC, MUC PM, console, plugin, vcard, xml-console), `win_page_up` sets `window->layout->paged = 1`. Subsequent `_win_printf` calls then silently drop every new message: ```c // src/ui/window.c, _win_printf if (window->layout->paged && wins_is_current(window)) { window->layout->unread_msg++; return; } ``` The reset back to `paged = 0` lives only inside the `WIN_CHAT`-gated DB-fetch block in `win_page_down` (lines 806–825): only `WIN_CHAT` can ever trigger `WIN_SCROLL_REACHED_BOTTOM`, which is the sole condition that flips `paged` off (line 847). For every other window type, `paged` stays `1` for the rest of the session — even after the user pages all the way back down to the bottom. This is the bug behind the "AI window stops showing user messages after scrolling up" report, and the same mechanism breaks new-message rendering in MUC and console after a single page-up. The existing `cl_ev_send_ai_msg` workaround on `feat/ai` (commit `010b2062a`) only patches the user's own outgoing AI message: it resets `paged` right before `win_print_outgoing`. Incoming AI streaming responses bypass `_win_printf` (they go through `win_println` directly), which is why those still show — but the `paged` mechanic itself remains broken for everything else. ## Fix Add a generic bottom-reached reset at the end of `win_page_down` for non-chat window types: ```c // Non-chat windows have no DB to fetch from, so WIN_SCROLL_REACHED_BOTTOM // never fires; reset paged once the buffer's last line is on screen. if (window->type != WIN_CHAT && (total_rows - *page_start) <= page_space + 1) { window->layout->paged = 0; window->layout->unread_msg = 0; } ``` The `+ 1` slack covers the past-end clamp branch a few lines above (`*page_start = total_rows - page_space - 1`), which lands `total_rows - *page_start` on `page_space + 1` even though the user is visually at the bottom. - It plugs the existing dispatch hole — `paged` was only reset on `WIN_CHAT`, this extends it to every other window type that uses `_win_printf` - Symmetric to the existing `WIN_SCROLL_REACHED_BOTTOM` reset two lines up; same semantics, just unconditional on window type - Fixes AI / MUC / MUC PM / console / vcard / plugin / xml-console all at once, not just AI - Makes the targeted reset in `cl_ev_send_ai_msg` (`010b2062a`) redundant — that commit can be reverted as cleanup once this lands ## Out of scope - Suppressed messages (those `_win_printf` dropped while `paged = 1`) are still lost: `_win_printf` only increments `unread_msg`, it does not buffer the dropped content. This is a pre-existing flaw shared with the `WIN_SCROLL_REACHED_BOTTOM` path and is not introduced by this change. Recovering them is a separate task (either keep the message in the buffer + render later, or display below a "new messages" marker). - The conceptual conflation of "buffer bottom" vs "DB bottom" in `win_scroll_state_t` for `WIN_CHAT` is not touched. ## Test plan - [ ] AI chat: send several messages → page up → page down → send a new message → message displays without needing the existing `cl_ev_send_ai_msg` reset - [ ] MUC: receive enough messages to scroll → page up → page down → peer's next message displays - [ ] MUC PM: same scenario as MUC - [ ] Console: scroll up through cons output → page down → new console output displays - [ ] Regular `WIN_CHAT`: page-up + page-down behaviour unchanged (still uses DB-fetch path; no double reset)

jabber.developer2 added 33 commits 2026-05-10 12:39:01 +00:00

feat(ai): add AI client with multi-provider support and UI ... e43e8378b0

Add an AI client module that integrates with OpenAI-compatible API
providers (OpenAI, Perplexity, and custom providers) to provide
AI-assisted responses within the profanity client.

The implementation includes:

- src/ai/ai_client.c/h: Core AI client with provider management,
  session handling, and async HTTP request handling via libcurl.
  Supports per-provider API keys stored in preferences, reference-
  counted sessions, and conversation history tracking.

- src/ui/window.c/window_list.c: New AI window type (ProfAiWin) for
  displaying AI conversations, with response streaming and error
  display capabilities.

- Command integration: New `/ai` command (cmd_defs.c, cmd_funcs.c)
  for creating sessions, sending prompts, and managing providers.
  Provider autocomplete support in cmd_ac.c.

- Preferences integration: API keys for providers are persisted in
  the preferences system (config/preferences.c).

- Unit tests: 472 lines of comprehensive tests covering provider
  management, session lifecycle, JSON escaping, and autocomplete
  (tests/unittests/test_ai_client.c).

Architecture decisions:
- Asynchronous design: HTTP requests run on a separate thread to
  avoid blocking the main UI loop. Callbacks are invoked on the main
  thread via direct function call (profanity uses ncurses, not GLib
  main loop).
- Reference counting: Both AIProvider and AISession use ref counting
  for safe shared ownership.
- Response size limit: 10MB cap on HTTP responses to prevent OOM.

fix(ai): set correct links in tests, fix headers 1f1770bd58

ref(ai): add stub, fix includes, move aiwin_send_message location 9c8ad57b59

ref(ai): move unfitting functions out of window.c; create aiwin.c cff05ca802

fix(ai): remove duplicate assistant response in the message history bccd3ecded

fix(ai): persist API keys to disk after set/remove operations ... 25e0459979

API keys set via /ai set token were only stored in memory and lost
on client restart. prefs_ai_set_token() and prefs_ai_remove_token()
mutated the in-memory prefs but never called _save_prefs(), unlike
other preference setters in the same file.

Both functions now call _save_prefs() after mutating the keyfile,
ensuring API keys persist across client restarts as advertised.

fix(ai): remove API key from debug log ... c9a5239117

Remove the log_debug line in ai_client.c that leaked the first 10
characters of the API key into logs. This is also contained a memory leak (g_strndup
return value was never freed).

fix(ai): remove redundant g_strdup on api_key assignment ... caa0b3ccba

ai_get_provider_key() already returns a freshly g_strdup'd copy of
the API key. The outer g_strdup in ai_session_create() created an
unnecessary second copy, leaking the original pointer.

fix(ai): align ai_list_providers with documented contract ... 93ad7379e2

Remove ai_provider_ref() from ai_list_providers() so the returned
providers do not need to be unref'd by the caller. This matches the
header docstring which states "caller must not free the list or
providers". Also update all callers (cmd_ai_providers, tests) to
remove redundant ai_provider_unref() calls.

fix(ai): fix provider_name leak and const-cast in cmd_ai_start ... 9e1f9b666e

Use auto_gchar temporary for g_strndup result in cmd_ai_start to
prevent memory leak. Fix const-cast warning by using a proper
non-const temporary variable.

fix(ai): join multi-word arguments in cmd_ai_correct ... da0bf43d73

Use g_strjoinv to join all arguments from args[1] onwards into a
single prompt string. Previously only args[1] was used, truncating
multi-word prompts like '/ai correct hello world' to just 'hello'

fix(ai): move generic subcommand autocomplete outside nested conditionals ... dc75f16221

The generic `/ai <subcommand>` tab-completion was nested inside conditional
blocks that prevented it from firing in common cases — for example,
`/ai<tab>` or `/ai <tab>` would never match subcommands because the
`autocomplete_param_with_ac()` call was guarded by argument count checks
and nested inside the explicit handler branches.

Move the fallback subcommand autocomplete to execute unconditionally after
all explicit handler branches, ensuring `/ai <tab>` always lists available
subcommands regardless of argument count or which subcommand was typed.

Also move the `/ai set` subcommand fallback outside the inner else-block
so it runs whenever args[0] is "set", not only when num_args >= 2.

refactor(ai): flatten autocomplete into sequential prefix-matching chain ... 002a6ed15b

Replace the deeply nested conditional tree in _ai_autocomplete() with a
flat sequential chain of autocomplete_param_with_*() calls. Each call
tries a specific command prefix (e.g., "/ai set provider", "/ai start")
and returns immediately on match — a common, reliable pattern for
command-line tab completion.

The previous design nested provider-name autocomplete inside
g_strv_length() and g_strcmp0() checks, meaning /ai<tab> and /ai <tab>
would never reach the subcommand matcher. The new flat structure ensures
every prefix is tried in order, with the most specific prefixes first
and the generic /ai subcommand fallback last.

This pattern — sequential prefix matching with early return — is the
standard approach for autocomplete dispatch: each handler owns its
prefix, no shared state or argument parsing is needed, and adding a
new subcommand is a single append to the chain.

refactor(ai): use stateful autocomplete for provider name matching ... 461c0c32dd

Replace the manual GHashTable iteration in ai_providers_find() with the
existing stateful autocomplete system. Provider names are now kept in
sync via autocomplete_add/remove during ai_add_provider/ai_remove_provider,
and ai_providers_find delegates to autocomplete_complete() for deterministic
tab-completion cycling through sorted results.

This eliminates manual GList allocation, iteration, and cleanup, while
providing consistent forward/backward cycling behavior shared by all
autocomplete callers.

fix(ai): standardize lock handling in _ai_invoke_callback ... 634fb7d7eb

Take the global lock mutex before invoking all callbacks (success
and error) to ensure thread-safety. Previously some paths took the
lock and others did not, creating asymmetric behavior that could
lead to data races when callbacks access UI state.

ref(ai): remove inaccurate logging 4913a3d5a4

fix(ai): guard NULL search_str and update tests for new cycling behavior ... 00f11eb704

Update ai_providers_find() to handle NULL search_str safely by treating it
as an empty string, preventing a segfault in strdup() within autocomplete_complete().

Rename test_ai_providers_find_case_sensitive to test_ai_providers_find_case_insensitive
to reflect the new case-insensitive matching contract. Update all affected tests to
expect cycling behavior (NULL/empty returns first provider) and use auto_gchar for
automatic memory management.

Use `gchar` instead of `char` in ai_providers_find for consistency

refactor(ai): remove redundant callback layer from AI client ... cead417e78

The ai_response_cb and ai_error_cb parameters were always passed the
same functions (aiwin_display_response and aiwin_display_error),
making the callback indirection redundant and complicating the call
chain.

Remove ai_callback_data_t, _ai_callback_invoke(), and
_ai_invoke_callback(). Simplify _ai_request_thread and ai_send_prompt
to directly call aiwin_display_error() and aiwin_display_response()
when user_data is a valid ProfAiWin*.

refactor(ai): add UI display helpers ... fe0a46da58

Add static _aiwin_display_error() and
_aiwin_display_response() helpers to reduce mutex repetition and
log warnings when aiwin is NULL or invalid.

fix(ai): validate aiwin pointer to prevent UAF when window closed ... 2fc7f3d672

Prevent use-after-free in the AI request worker thread when the user
closes the AI window during the 60-second HTTP request timeout. Without
this fix, the worker may dereference a dangling pointer and crash or
exhibit undefined behavior.

The _ai_request_thread function in ai_client.c previously cast user_data
to ProfAiWin* and used it directly without verifying the window was still
alive. Added wins_ai_exists() to iterate through all AI windows and check
if the pointer is still valid before use.

Safety:
- wins_ai_exists() iterates all AI windows (handles multiple AI windows)
- Worker skips UI update if window was freed, just cleans up
- Logs warning when dangling pointer is detected

feat(ai): disable request storage via store:false parameter ... cfe6ea46e2

Add store:false to AI API requests to prevent OpenAI, Perplexity, and
other providers from storing conversation data or using it for training.

Both the OpenAI Responses API and Perplexity API support the store
parameter to control whether requests are persisted. Setting it to false
ensures conversations remain private and are not retained by the provider.

Privacy:
- Requests are not stored by AI providers
- Conversations are not used for model training
- Aligns with CProof's privacy-first design

fix(ai): convert \n escape sequences to actual newlines in LLM responses a16237d16b

fix(ai): use atomic operations for refcounting ... f133d81a05

Replace plain ref_count++/-- with g_atomic_int_inc and
g_atomic_int_dec_and_test for both AIProvider and AISession refcounts.
Prevents data races when ref/unref is called from worker threads.

ref(pref): add newline at the end of the file cdcc45b7c6

fix(ai): fix leak from ai_get_provider_key f4221e27ac

fix(ai): return CURL_WRITEFUNC_ERROR on oversized response ... e229ed1281

Return CURL_WRITEFUNC_ERROR instead of realsize when the response
exceeds 10MB. This causes curl to abort the transfer immediately
instead of continuing to stream data.

Merge branch 'master' into feat/ai 731b55fa19

feat(ai): add model caching, settings, and commands ... acae543057

- Introduce model caching with persistence to preferences
- Add provider default model and custom settings management
- Implement `/ai switch`, `/ai models`, and improve `/ai start`
- Add model name autocomplete for chat commands
- Update command definitions and help text
- Add unit tests for new functionality

fix(ai): protect window validation with mutex ... 07d267b5bc

The existence check accesses shared state without synchronization,
which can cause race conditions when called from background threads.
Explicitly locking the mutex ensures safe access during validation.

feat(ai): implement robust JSON model parsing ... a898cb212d

Rewrote internal JSON parsing to correctly handle whitespace, escaped
quotes, and strict field context extraction. This prevents incorrect
field names or provider names from being added to the model list.

Added `ai_parse_models_from_json` public API to facilitate testing.

Changed `/ai models` default behavior to always fetch fresh models.
Replaced `--refresh` flag with `--cached` to display local cache.

Added comprehensive unit tests covering OpenAI and Perplexity formats,
array format, empty/null JSON, escaped quotes, and whitespace handling.

fix(ai): improve API error parsing and fallback ... ecb07fd00c

Add early return for empty responses and fallback to console output
when the AI window is closed or invalid. Implement JSON error
parsing to extract readable messages from API failures, improving
debugging and user feedback.

fix(ai): reset paged flag to display user message ... 010b2062a4

When the user scrolls up to view history, the paged flag is set to 1.
This suppresses new messages in _win_printf(). Reset paged and
unread_msg flags before printing to ensure visibility.

fix(ui): reset paged flag at buffer bottom for non-chat windows c167f48714

jabber.developer2 changed target branch from master to feat/ai 2026-05-10 13:26:40 +00:00

jabber.developer2 changed title from fix(ui): reset paged flag at buffer bottom for non-chat windows to WIP: fix(ui): reset paged flag at buffer bottom for non-chat windows 2026-05-10 13:31:32 +00:00

jabber.developer force-pushed fix/paged-non-chat-windows from c167f48714 to e841e6ac0c 2026-05-15 12:22:35 +00:00 Compare

jabber.developer changed target branch from feat/ai to master 2026-05-15 12:24:19 +00:00

jabber.developer changed title from WIP: fix(ui): reset paged flag at buffer bottom for non-chat windows to fix(ui): reset paged flag at buffer bottom for non-chat windows 2026-05-15 14:29:07 +00:00

jabber.developer force-pushed fix/paged-non-chat-windows from e841e6ac0c to 0096b11d24 2026-05-15 14:32:41 +00:00 Compare

jabber.developer2 closed this pull request 2026-05-15 16:52:11 +00:00

All checks were successful

Hide all checks

CI Code / Check spelling (pull_request) Successful in 16s

Required

Details

CI Code / Check coding style (pull_request) Successful in 31s

Required

Details

CI Code / Code Coverage (pull_request) Successful in 2m43s

Required

Details

CI Code / Linux (debian) (pull_request) Successful in 4m43s

Required

Details

CI Code / Linux (ubuntu) (pull_request) Successful in 4m56s

Required

Details

CI Code / Linux (arch) (pull_request) Successful in 5m48s

Required

Details

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

Reviewers

No Reviewers

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

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

admin jabber.developer jabber.developer2

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: devs/cproof#117

No description provided.