Skip to content

Commit 285849e

Browse files
authored
Merge pull request #5 from Web-Dev-Codi/askQ
Ask q
2 parents 9c55fff + 62348f1 commit 285849e

13 files changed

Lines changed: 694 additions & 208 deletions

README.md

Lines changed: 171 additions & 102 deletions
Large diffs are not rendered by default.

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ license = { text = "MIT" }
1111
readme = "README.md"
1212
requires-python = ">=3.11"
1313
dependencies = [
14-
"textual>=8.0.0",
14+
"textual>=8.0.1",
1515
"ollama>=0.6.1",
1616
"pydantic>=2.12.5",
1717
"rich>=14.3.2",

src/ollama_chat/app.py

Lines changed: 144 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
ConversationPickerScreen,
5353
ImageAttachScreen,
5454
InfoScreen,
55-
QuestionScreen,
5655
SimplePickerScreen,
5756
TextPromptScreen,
5857
ThemePickerScreen,
@@ -70,6 +69,7 @@
7069
)
7170
from .tools.base import ToolContext
7271
from .widgets.activity_bar import ActivityBar
72+
from .widgets.ask_question_widget import AskQuestionWidget
7373
from .widgets.conversation import ConversationView
7474
from .widgets.input_box import InputBox
7575
from .widgets.message import MessageBubble
@@ -473,6 +473,10 @@ def __init__(self) -> None:
473473
self._w_activity: ActivityBar | None = None
474474
self._w_status: StatusBar | None = None
475475
self._w_conversation: ConversationView | None = None
476+
self._w_input_box: InputBox | None = None
477+
self._w_ask_question: AskQuestionWidget | None = None
478+
479+
self._question_answer_queue: asyncio.Queue[str | None] = asyncio.Queue()
476480

477481
self._slash_commands: list[tuple[str, str]] = [
478482
("/image <path>", "Attach image from filesystem"),
@@ -802,6 +806,7 @@ def compose(self) -> ComposeResult:
802806
with Container(id="app-root"):
803807
yield ConversationView(id="conversation")
804808
yield InputBox()
809+
yield AskQuestionWidget()
805810
yield StatusBar(id="status_bar")
806811
yield ActivityBar(
807812
shortcut_hints="ctrl+p commands",
@@ -931,6 +936,8 @@ async def on_mount(self) -> None:
931936
self._w_activity = self.query_one("#activity_bar", ActivityBar)
932937
self._w_status = self.query_one("#status_bar", StatusBar)
933938
self._w_conversation = self.query_one(ConversationView)
939+
self._w_input_box = self.query_one(InputBox)
940+
self._w_ask_question = self.query_one(AskQuestionWidget)
934941

935942
attach_button = self.query_one("#attach_button", Button)
936943
self._w_input.disabled = True
@@ -1173,32 +1180,53 @@ def _on_support_file_event(self, event: str, payload: dict[str, Any]) -> None:
11731180
def _on_question_asked(self, event_name: str, payload: dict[str, Any]) -> None:
11741181
"""Handle question.asked event from question_service.
11751182
1176-
Uses run_worker so the coroutine runs on Textual's main event loop
1177-
with a proper worker context — required by push_screen_wait, and safe
1178-
to call from any thread (including the tool-executor thread that fires
1179-
this callback via _run_async_from_sync).
1183+
Schedules _run_question_sequence on Textual's main event loop via
1184+
call_from_thread + run_worker. The question_service.ask() call may run
1185+
in a background thread (tools are executed via asyncio.to_thread), so
1186+
this handler must not call run_worker directly from that thread.
11801187
"""
1181-
self.run_worker(self._run_question_sequence(payload))
1188+
def _start_question_worker() -> None:
1189+
self.run_worker(self._run_question_sequence(payload))
1190+
1191+
# call_from_thread is thread-safe and will invoke the closure on the
1192+
# main Textual thread, regardless of which thread published the event.
1193+
try:
1194+
self.call_from_thread(_start_question_worker)
1195+
except RuntimeError:
1196+
# Fallback for call sites already running on the app loop.
1197+
self.run_worker(self._run_question_sequence(payload))
1198+
except Exception as exc: # pragma: no cover - defensive logging
1199+
LOGGER.debug(
1200+
"app.question_worker.start_failed",
1201+
extra={"event": event_name, "error": str(exc)},
1202+
)
11821203

11831204
async def _run_question_sequence(self, payload: dict[str, Any]) -> None:
1184-
"""Show QuestionScreen modals sequentially and reply to question_service."""
1205+
"""Show inline AskQuestionWidget sequentially and reply to question_service."""
11851206
qid: str = payload.get("id", "")
11861207
questions: list[dict[str, Any]] = payload.get("questions", [])
11871208
all_answers: list[list[str]] = []
11881209

11891210
for q in questions:
11901211
try:
1191-
result: list[str] | None = await self.push_screen_wait(
1192-
QuestionScreen(q)
1212+
result = await self._ask_inline_question(q)
1213+
except asyncio.CancelledError:
1214+
LOGGER.debug(
1215+
"app.question_sequence.cancelled",
1216+
extra={"qid": qid},
11931217
)
1218+
break
11941219
except Exception as exc:
11951220
LOGGER.debug(
1196-
"app.question_screen.failed",
1221+
"app.question_inline.failed",
11971222
extra={"qid": qid, "error": str(exc)},
11981223
)
11991224
result = None
12001225
all_answers.append(result if result is not None else [])
12011226

1227+
if len(all_answers) < len(questions):
1228+
all_answers.extend([[] for _ in range(len(questions) - len(all_answers))])
1229+
12021230
try:
12031231
question_service.reply(qid, all_answers)
12041232
except Exception as exc:
@@ -1207,6 +1235,104 @@ async def _run_question_sequence(self, payload: dict[str, Any]) -> None:
12071235
extra={"qid": qid, "error": str(exc)},
12081236
)
12091237

1238+
def _show_question_widget(self) -> None:
1239+
input_box = self._w_input_box or self.query_one(InputBox)
1240+
ask_widget = self._w_ask_question or self.query_one(AskQuestionWidget)
1241+
input_box.display = False
1242+
ask_widget.display = True
1243+
1244+
def _focus_question_controls() -> None:
1245+
try:
1246+
options = ask_widget.query_one("#aq-options", OptionList)
1247+
if options.option_count:
1248+
options.focus()
1249+
return
1250+
except Exception:
1251+
pass
1252+
1253+
try:
1254+
custom_input = ask_widget.query_one("#aq-custom-input", Input)
1255+
if custom_input.display:
1256+
custom_input.focus()
1257+
return
1258+
except Exception:
1259+
pass
1260+
1261+
try:
1262+
ask_widget.focus()
1263+
except Exception:
1264+
return
1265+
1266+
self.call_after_refresh(_focus_question_controls)
1267+
1268+
def _restore_input(self) -> None:
1269+
ask_widget = self._w_ask_question or self.query_one(AskQuestionWidget)
1270+
input_box = self._w_input_box or self.query_one(InputBox)
1271+
ask_widget.display = False
1272+
input_box.display = True
1273+
input_widget = self._w_input or self.query_one("#message_input", Input)
1274+
input_widget.focus()
1275+
1276+
def _drain_answer_queue(self) -> None:
1277+
while True:
1278+
try:
1279+
self._question_answer_queue.get_nowait()
1280+
except asyncio.QueueEmpty:
1281+
return
1282+
1283+
async def _ask_inline_question(self, question_info: dict[str, Any]) -> list[str] | None:
1284+
question = str(question_info.get("question", "")).strip()
1285+
header = str(question_info.get("header", "Assistant Question")).strip()
1286+
options_raw = question_info.get("options")
1287+
custom = bool(question_info.get("custom", True))
1288+
1289+
options: list[str] = []
1290+
if isinstance(options_raw, list):
1291+
for item in options_raw:
1292+
if isinstance(item, dict):
1293+
label = str(item.get("label", "")).strip()
1294+
if label:
1295+
options.append(label)
1296+
elif isinstance(item, str):
1297+
label = item.strip()
1298+
if label:
1299+
options.append(label)
1300+
1301+
if not question:
1302+
return []
1303+
1304+
ask_widget = self._w_ask_question or self.query_one(AskQuestionWidget)
1305+
ask_widget.border_title = header or "Assistant Question"
1306+
1307+
self._drain_answer_queue()
1308+
ask_widget.load_question(question, options, custom=custom)
1309+
self._show_question_widget()
1310+
1311+
try:
1312+
answer = await self._question_answer_queue.get()
1313+
finally:
1314+
try:
1315+
self._restore_input()
1316+
except Exception as exc:
1317+
LOGGER.debug(
1318+
"app.question.restore_input_failed",
1319+
extra={"error": str(exc)},
1320+
)
1321+
1322+
if answer is None:
1323+
return []
1324+
1325+
value = str(answer).strip()
1326+
if not value:
1327+
return []
1328+
return [value]
1329+
1330+
async def on_ask_question_widget_answered(
1331+
self, message: AskQuestionWidget.Answered
1332+
) -> None:
1333+
message.stop()
1334+
self._question_answer_queue.put_nowait(message.value)
1335+
12101336
@property
12111337
def show_timestamps(self) -> bool:
12121338
return bool(self.config["ui"]["show_timestamps"])
@@ -1556,6 +1682,14 @@ def _scroll() -> None:
15561682

15571683
async def action_interrupt_stream(self) -> None:
15581684
"""Cancel an in-flight assistant response (delegates to StreamManager)."""
1685+
try:
1686+
ask_widget = self._w_ask_question or self.query_one(AskQuestionWidget)
1687+
if ask_widget.display:
1688+
self._question_answer_queue.put_nowait(None)
1689+
return
1690+
except Exception:
1691+
pass
1692+
15591693
interrupted = await self.stream_manager.interrupt_stream(self.chat.model)
15601694
if interrupted:
15611695
self._update_status_bar()

src/ollama_chat/capabilities.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ def from_config(cls, config: dict[str, Any]) -> CapabilityContext:
5050
show_thinking=bool(cap_cfg.get("show_thinking", True)),
5151
web_search_enabled=bool(cap_cfg.get("web_search_enabled", False)),
5252
web_search_api_key=str(cap_cfg.get("web_search_api_key", "")),
53-
max_tool_iterations=int(cap_cfg.get("max_tool_iterations", 10)),
53+
max_tool_iterations=int(cap_cfg.get("max_tool_iterations", 20)),
5454
)
5555

5656

src/ollama_chat/chat.py

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
_QUESTION_USE_POLICY = """
5757
5858
CLARIFYING QUESTIONS POLICY:
59-
You have access to a 'question' tool. Use it proactively when:
59+
You have access to an 'ask_user_question' tool. Use it proactively when:
6060
6161
ALWAYS ASK WHEN:
6262
• User request is ambiguous ("fix the bug", "add auth", "optimize code")
@@ -77,13 +77,10 @@
7777
• Questions should be answerable in <10 seconds
7878
7979
FORMAT REQUIREMENTS (CRITICAL):
80-
• Each question MUST have a 'header' field (short label, max 30 chars)
81-
• Each question MUST have a 'question' field (full question text)
82-
• 'options' MUST be a list of objects, NOT strings
83-
• Each option MUST have both 'label' and 'description' fields
84-
• Example: {"label": "Redis", "description": "Fast, external, scalable"}
85-
• WRONG: ["Redis", "Memcached"] ❌
86-
• CORRECT: [{"label": "Redis", "description": "Fast, external"}, ...] ✓
80+
• The tool call MUST include a 'question' string
81+
• The tool call MUST include an 'options' list of 2-5 strings
82+
• WRONG: {"options": [{"label": "Redis"}]} ❌
83+
• CORRECT: {"options": ["Redis", "Memcached"]} ✓
8784
8885
WHEN NOT TO ASK (rare):
8986
• Task is completely unambiguous ("write factorial function")
@@ -102,7 +99,7 @@
10299
Example 1 - Ambiguous Code Target:
103100
User: "Refactor the database connection code"
104101
You (thinking): "Multiple files handle DB connections. Need to ask which."
105-
You (action): Call question tool with:
102+
You (action): Call ask_user_question tool with:
106103
question: "Which database connection code should I refactor?"
107104
options: [
108105
"Main connection pool (db/pool.py)",
@@ -116,7 +113,7 @@
116113
Example 2 - Technology Choice:
117114
User: "Add caching to the API endpoints"
118115
You (thinking): "Many caching strategies exist. Should ask."
119-
You (action): Call question tool with:
116+
You (action): Call ask_user_question tool with:
120117
question: "Which caching backend should I use?"
121118
options: [
122119
"Redis (fast, external, scalable)",
@@ -143,8 +140,10 @@ def factorial(n: int) -> int:
143140

144141
# Tools that are I/O-bound and fast - don't need thread pool overhead
145142
# These tools complete quickly (<10ms) and don't block the event loop.
146-
# NOTE: Tools that may wait on user interaction (e.g. "question") MUST NOT be
147-
# listed here, otherwise they will block the Textual event loop and freeze the UI.
143+
# NOTE: Tools that may wait on user interaction (e.g. ask_user_question) MUST NOT be
144+
# listed here. The tool execution path is synchronous at this layer; running an
145+
# interactive tool on the main event loop would block Textual from rendering the
146+
# question UI and handling input.
148147
FAST_SYNC_TOOLS = {
149148
"read",
150149
"write",
@@ -763,9 +762,10 @@ async def _stream_once_with_capabilities(
763762
# Build policy text - start with base tool use policy
764763
policy_text = _TOOL_USE_POLICY
765764

766-
# Check if question tool is available in this request
765+
# Check if ask_user_question tool is available in this request
767766
has_question_tool = any(
768-
t.get("function", {}).get("name") == "question" for t in formatted_tools
767+
t.get("function", {}).get("name") == "ask_user_question"
768+
for t in formatted_tools
769769
)
770770

771771
# Add question-specific guidance if tool is present

src/ollama_chat/tools/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
"batch_tool",
2323
"lsp_tool",
2424
"plan_tool",
25-
"question_tool",
25+
"ask_user_question_tool",
2626
"todo_tool",
2727
"skill_tool",
2828
"apply_patch_tool",
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
from __future__ import annotations
2+
3+
from pydantic import Field
4+
5+
from ..support import question_service
6+
from .base import ParamsSchema, Tool, ToolContext, ToolResult
7+
8+
9+
class AskUserQuestionParams(ParamsSchema):
10+
question: str = Field(description="The question to present to the user")
11+
options: list[str] = Field(
12+
description="List of 2–5 answer options to display",
13+
min_length=2,
14+
max_length=5,
15+
)
16+
17+
18+
class AskUserQuestionTool(Tool):
19+
id = "ask_user_question"
20+
description = (
21+
"Ask the user a multiple-choice question when you need clarification before proceeding. "
22+
"Use this instead of guessing. Provide 2 to 5 clear, distinct options. "
23+
"The user may also type a custom answer."
24+
)
25+
params_schema = AskUserQuestionParams
26+
27+
async def execute(
28+
self, params: AskUserQuestionParams, ctx: ToolContext
29+
) -> ToolResult:
30+
question = (params.question or "").strip()
31+
options = [str(opt).strip() for opt in (params.options or []) if str(opt).strip()]
32+
33+
if not question:
34+
return ToolResult(
35+
title="Question",
36+
output="User did not answer.",
37+
metadata={"answer": None},
38+
)
39+
40+
if len(options) < 2:
41+
return ToolResult(
42+
title="Question",
43+
output="User did not answer.",
44+
metadata={"answer": None},
45+
)
46+
47+
questions = [
48+
{
49+
"header": "Assistant Question",
50+
"question": question,
51+
"options": [{"label": opt, "description": ""} for opt in options],
52+
"multiple": False,
53+
"custom": True,
54+
}
55+
]
56+
57+
answers = await question_service.ask(session_id=ctx.session_id, questions=questions)
58+
chosen: str | None = None
59+
if answers and answers[0]:
60+
chosen = str(answers[0][0]).strip() if str(answers[0][0]).strip() else None
61+
62+
if not chosen:
63+
return ToolResult(
64+
title="Question",
65+
output="User did not answer.",
66+
metadata={"answer": None},
67+
)
68+
69+
return ToolResult(
70+
title="Question",
71+
output=chosen,
72+
metadata={"answer": chosen},
73+
)

0 commit comments

Comments
 (0)