Merge pull request #3560 from lightpanda-io/agent-repl-simplify

agent: REPL output fixes and command dispatch simplification

src/agent/Agent.zig

@@ -194,14 +194,8 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent
     var providers_buf: [@typeInfo(Config.AiProvider).@"enum".fields.len]Candidate = undefined;
     const found_providers = settings.availableProviders(&providers_buf);
     const available_providers = try allocator.alloc([]const u8, found_providers.len);
-    var provider_count: usize = 0;
-    errdefer {
-        for (available_providers[0..provider_count]) |p| allocator.free(p);
-        allocator.free(available_providers);
-    }
     for (found_providers, 0..) |f, i| {
-        available_providers[i] = try allocator.dupe(u8, @tagName(f.provider));
-        provider_count = i + 1;
+        available_providers[i] = @tagName(f.provider);
     }
 
     if (opts.task != null and opts.script_file != null) {
@@ -282,13 +276,9 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent
     if (resolved) |*r| if (!will_repl) {
         const remembered_matches = remembered != null and remembered.?.provider == r.credential.provider;
         const explicit = opts.model != null or remembered_matches;
-        switch (try settings.reconcileModel(allocator, &r.credential, model, opts.base_url, explicit)) {
-            .use => |m| {
-                allocator.free(model);
-                model = m;
-            },
-            .abort => return error.ModelNotAvailable,
-        }
+        const resolved_model = try settings.reconcileModel(allocator, &r.credential, model, opts.base_url, explicit);
+        allocator.free(model);
+        model = resolved_model;
     };
 
     const effort = settings.resolveEffort(opts, remembered, will_repl, if (resolved) |r| r.credential.provider else null);
@@ -373,7 +363,6 @@ pub fn deinit(self: *Agent) void {
     if (self.ai_client) |ai_client| ai_client.deinit(self.allocator);
     if (self.credential) |*c| c.deinit(self.allocator);
     self.allocator.free(self.model);
-    for (self.available_providers) |p| self.allocator.free(p);
     self.allocator.free(self.available_providers);
     self.allocator.destroy(self);
 }
@@ -678,9 +667,9 @@ fn runRepl(self: *Agent) void {
             },
             .tool_call => |tc| {
                 self.terminal.beginTool(tc.name(), slash_split.?.rest);
-                const result = self.runCommand(aa, cmd);
+                const result = self.runCommand(aa, tc);
                 self.terminal.endTool();
-                self.printCommandResult(cmd, result);
+                self.printCommandResult(tc, result);
                 if (!result.is_error) {
                     self.recordSaveCommand(navigationGoto(aa, tc.tool, tc.args) orelse cmd);
                 }
@@ -1123,33 +1112,18 @@ fn handleSave(self: *Agent, arena: std.mem.Allocator, rest: []const u8) void {
         self.terminal.printWarning("prompt ignored without an LLM; saving the recorded commands as-is", .{});
     }
     const resolved = self.resolveSavePathAndMode(arena, parsed.filename) orelse return;
-    const path = resolved.path;
-    const mode = resolved.mode;
 
-    // `path` aliases either an arena-owned string (first save) or
-    // `self.save_path` (subsequent saves to the same destination); only the
-    // former needs persisting into agent-owned memory.
-    var new_save_path: ?[]u8 = if (self.save_path == null)
-        self.allocator.dupe(u8, path) catch |err| {
-            self.terminal.printError("failed to remember save destination {s}: {s}", .{ path, @errorName(err) });
-            return;
-        }
-    else
-        null;
-    defer if (new_save_path) |p| self.allocator.free(p);
-
-    save.writeContentFile(path, self.save_buffer.bytes(), mode) catch |err| {
-        self.terminal.printError("failed to save {s}: {s}", .{ path, @errorName(err) });
+    save.writeContentFile(resolved.path, self.save_buffer.bytes(), resolved.mode) catch |err| {
+        self.terminal.printError("failed to save {s}: {s}", .{ resolved.path, @errorName(err) });
         return;
     };
 
-    if (new_save_path) |p| {
-        self.save_path = p;
-        new_save_path = null;
-    }
+    self.rememberSavePath(resolved.path) catch |err| {
+        self.terminal.printWarning("failed to remember save destination {s}: {s}", .{ resolved.path, @errorName(err) });
+    };
     const saved_lines = self.save_buffer.lines;
     self.save_buffer.reset();
-    self.terminal.printInfo("Saved {d} line(s) to {s}", .{ saved_lines, self.save_path.? });
+    self.terminal.printInfo("Saved {d} line(s) to {s}", .{ saved_lines, resolved.path });
 }
 
 fn promptSaveMode(self: *Agent, path: []const u8) ?save.Mode {
@@ -1313,17 +1287,19 @@ fn synthesizeSaveTo(self: *Agent, arena: std.mem.Allocator, path: []const u8, mo
         return;
     };
 
-    self.rememberSavePath(path);
+    self.rememberSavePath(path) catch |err| {
+        self.terminal.printWarning("failed to remember save destination {s}: {s}", .{ path, @errorName(err) });
+    };
     self.save_buffer.reset();
     self.terminal.printInfo("Saved synthesized script to {s}", .{path});
 }
 
 /// Persist `path` as the destination reused by a subsequent bare `/save`.
-fn rememberSavePath(self: *Agent, path: []const u8) void {
+fn rememberSavePath(self: *Agent, path: []const u8) !void {
     if (self.save_path) |old| {
         if (std.mem.eql(u8, old, path)) return;
     }
-    const dup = self.allocator.dupe(u8, path) catch return;
+    const dup = try self.allocator.dupe(u8, path);
     if (self.save_path) |old| self.allocator.free(old);
     self.save_path = dup;
 }
@@ -1466,13 +1442,7 @@ fn printSlashHelp(self: *Agent, arena: std.mem.Allocator, target: []const u8) vo
     self.terminal.printInfo("/{s} — {s}", .{ tool_schema.tool_name, tool_schema.description });
 }
 
-/// Caller contract: `cmd` must be `.tool_call` — `.comment` and `.llm` are
-/// filtered upstream, having no tool mapping.
-fn runCommand(self: *Agent, arena: std.mem.Allocator, cmd: Command) browser_tools.ToolResult {
-    const tc = switch (cmd) {
-        .tool_call => |t| t,
-        else => return .{ .text = "internal: command has no tool mapping", .is_error = true },
-    };
+fn runCommand(self: *Agent, arena: std.mem.Allocator, tc: Command.ToolCall) browser_tools.ToolResult {
     // The terminal can't show an image, but the conversation can.
     return browser_tools.call(arena, self.ts.session, &self.ts.registry, tc.name(), tc.args, .{ .inline_image = self.ai_client != null }) catch |err| .{
         .text = switch (err) {
@@ -1487,14 +1457,9 @@ fn runCommand(self: *Agent, arena: std.mem.Allocator, cmd: Command) browser_tool
 /// Data output (/extract, /evaluate, /markdown, /tree, …) → plain stdout on
 /// success so a caller can pipe it. Everything else routes through
 /// `printToolOutcome`, which lays down the green ● / red ● dot shared with the
-/// LLM tool-call path. Callers only invoke this for `.tool_call` commands (the
-/// comment/login/acceptCookies branches take other paths).
-fn printCommandResult(self: *Agent, cmd: Command, result: browser_tools.ToolResult) void {
-    const tc = switch (cmd) {
-        .tool_call => |t| t,
-        else => return,
-    };
-    if (cmd.producesData() and !result.is_error) {
+/// LLM tool-call path.
+fn printCommandResult(self: *Agent, tc: Command.ToolCall, result: browser_tools.ToolResult) void {
+    if (tc.tool.producesData() and !result.is_error) {
         self.printData(tc.tool, result.text);
         return;
     }
@@ -1858,26 +1823,23 @@ fn buildUserMessageParts(
             return error.UnsupportedAttachment;
         };
 
-        if (std.mem.startsWith(u8, mime, "text/")) {
-            const bytes = std.Io.Dir.cwd().readFileAlloc(lp.io, path, ma, .limited(512 * 1024)) catch |err| {
-                log.err(.app, "read attachment failed", .{ .path = path, .err = err });
-                self.terminal.printError("could not read attachment: {s}", .{path});
-                return error.AttachmentReadFailed;
-            };
+        const is_text = std.mem.startsWith(u8, mime, "text/");
+        const limit: usize = if (is_text) 512 * 1024 else 20 * 1024 * 1024;
+        const content = std.Io.Dir.cwd().readFileAlloc(lp.io, path, ma, .limited(limit)) catch |err| {
+            log.err(.app, "read attachment failed", .{ .path = path, .err = err });
+            self.terminal.printError("could not read attachment: {s}", .{path});
+            return error.AttachmentReadFailed;
+        };
+
+        if (is_text) {
             try text_prefix.print(
                 ma,
                 "[Attached file: {s}]\n{s}\n[End of attachment]\n\n",
-                .{ path, bytes },
+                .{ path, content },
             );
         } else {
-            const raw = std.Io.Dir.cwd().readFileAlloc(lp.io, path, ma, .limited(20 * 1024 * 1024)) catch |err| {
-                log.err(.app, "read attachment failed", .{ .path = path, .err = err });
-                self.terminal.printError("could not read attachment: {s}", .{path});
-                return error.AttachmentReadFailed;
-            };
-            const b64_len = std.base64.standard.Encoder.calcSize(raw.len);
-            const b64 = try ma.alloc(u8, b64_len);
-            _ = std.base64.standard.Encoder.encode(b64, raw);
+            const b64 = try ma.alloc(u8, std.base64.standard.Encoder.calcSize(content.len));
+            _ = std.base64.standard.Encoder.encode(b64, content);
             try inline_parts.append(ma, .{ .image = .{
                 .data = b64,
                 .mime_type = try ma.dupe(u8, mime),
@@ -1885,11 +1847,9 @@ fn buildUserMessageParts(
         }
     }
 
-    var parts: std.ArrayList(zenai.provider.ContentPart) = .empty;
     try text_prefix.appendSlice(ma, user_input);
-    try parts.append(ma, .{ .text = try text_prefix.toOwnedSlice(ma) });
-    for (inline_parts.items) |p| try parts.append(ma, p);
-    return parts.toOwnedSlice(ma);
+    try inline_parts.insert(ma, 0, .{ .text = try text_prefix.toOwnedSlice(ma) });
+    return inline_parts.toOwnedSlice(ma);
 }
 
 // Tool results are re-sent with every subsequent turn, so an unscoped read of
@@ -2015,9 +1975,7 @@ fn completionProviders(context: *anyopaque, arena: std.mem.Allocator) []const []
         if (reachable[i]) extra += 1;
     }
     const names = arena.alloc([]const u8, self.available_providers.len + auth.registry.len + 1 + extra) catch return &.{};
-    for (self.available_providers, 0..) |p, i| {
-        names[i] = arena.dupe(u8, p) catch return &.{};
-    }
+    @memcpy(names[0..self.available_providers.len], self.available_providers);
     var n = self.available_providers.len;
     // Subscription providers complete even without a stored token — selecting
     // one is what starts the login.

src/agent/SlashCommand.zig

@@ -88,25 +88,21 @@ pub fn findMeta(name: []const u8) ?*const MetaCommand {
 const browser_tools = lp.tools;
 const llm_values = std.enums.values(Command.LlmCommand);
 
-/// Every slash-invocable name: browser tools, LLM triggers, meta commands.
-pub const all_names: [browser_tools.names.len + meta_commands.len + llm_values.len][]const u8 = blk: {
-    var arr: [browser_tools.names.len + meta_commands.len + llm_values.len][]const u8 = undefined;
-    var idx: usize = 0;
-    for (browser_tools.names) |n| {
-        arr[idx] = n;
-        idx += 1;
-    }
-    for (llm_values) |lc| {
-        arr[idx] = @tagName(lc);
-        idx += 1;
-    }
-    for (meta_commands) |m| {
-        arr[idx] = m.name;
-        idx += 1;
-    }
+const llm_names = blk: {
+    var arr: [llm_values.len][]const u8 = undefined;
+    for (llm_values, &arr) |v, *slot| slot.* = @tagName(v);
     break :blk arr;
 };
 
+const meta_names = blk: {
+    var arr: [meta_commands.len][]const u8 = undefined;
+    for (meta_commands, &arr) |m, *slot| slot.* = m.name;
+    break :blk arr;
+};
+
+/// Every slash-invocable name: browser tools, LLM triggers, meta commands.
+pub const all_names = browser_tools.names ++ llm_names ++ meta_names;
+
 /// Closest command name within two edits, or null — for "did you mean?" on typos.
 pub fn closestCommand(name: []const u8) ?[]const u8 {
     return string.closest(name, &all_names, 2);

src/agent/Terminal.zig

@@ -18,6 +18,7 @@
 
 const std = @import("std");
 const lp = @import("lightpanda");
+const string = @import("../string.zig");
 const Config = lp.Config;
 const Schema = lp.Schema;
 const SlashCommand = @import("SlashCommand.zig");
@@ -117,7 +118,7 @@ pub fn agentToolDone(self: *Terminal, name: []const u8, args: []const u8, ok: bo
         const a = if (self.repl_arena) |*ra| ra else return;
         defer _ = a.reset(.retain_capacity);
         const bytes = formatBulletLine(a.allocator(), name, args, ok) catch return;
-        _ = self.spinner.emitAbove(bytes);
+        self.emitStderr(bytes);
         return;
     }
     if (self.stderr_is_tty) {
@@ -131,12 +132,16 @@ pub fn agentToolDone(self: *Terminal, name: []const u8, args: []const u8, ok: bo
     }
 }
 
+/// Commit a finished line above the spinner, or straight to stderr when the
+/// spinner isn't running (non-tty REPL) so the line isn't silently dropped.
+fn emitStderr(self: *Terminal, bytes: []const u8) void {
+    if (self.spinner.emitAbove(bytes)) return;
+    _ = std.c.write(std.posix.STDERR_FILENO, bytes.ptr, bytes.len);
+}
+
 fn formatBulletLine(arena: std.mem.Allocator, name: []const u8, args: []const u8, ok: bool) ![]const u8 {
-    var aw: std.Io.Writer.Allocating = .init(arena);
-    const w = &aw.writer;
     const bullet_color = if (ok) ansi.green else ansi.red;
-    try w.print(bullet_line_fmt, .{ bullet_color, ansi.reset, ansi.dim, name, ansi.reset, args });
-    return aw.written();
+    return std.fmt.allocPrint(arena, bullet_line_fmt, .{ bullet_color, ansi.reset, ansi.dim, name, ansi.reset, args });
 }
 
 pub fn setIdleCallback(fun: ?*const c.ic_idle_fun_t, arg: ?*anyopaque) void {
@@ -286,12 +291,10 @@ pub fn printToolOutcome(self: *Terminal, name: []const u8, text: []const u8, is_
     if (self.repl_arena) |*a| {
         defer _ = a.reset(.retain_capacity);
         const bytes = formatReplOutcome(a.allocator(), text, is_error) catch return;
-        if (self.spinner.emitAbove(bytes)) return;
-        _ = std.c.write(std.posix.STDERR_FILENO, (bytes).ptr, (bytes).len);
-        return;
+        return self.emitStderr(bytes);
     }
     if (!is_error and !self.verbosity.atLeast(.medium)) return;
-    const truncated = text[0..@min(text.len, max_result_display_len)];
+    const truncated = string.truncateUtf8(text, max_result_display_len);
     const ellipsis: []const u8 = if (text.len > max_result_display_len) "..." else "";
     const color: []const u8 = if (is_error) ansi.red else ansi.green;
     std.debug.print("{s}{s}[result: {s}]{s} {s}{s}\n", .{ ansi.dim, color, name, ansi.reset, truncated, ellipsis });
@@ -351,12 +354,8 @@ pub fn printWarning(self: *Terminal, comptime fmt: []const u8, args: anytype) vo
 fn printSeverity(self: *Terminal, color: []const u8, label: []const u8, comptime fmt: []const u8, args: anytype) void {
     if (self.repl_arena) |*a| {
         defer _ = a.reset(.retain_capacity);
-        var aw: std.Io.Writer.Allocating = .init(a.allocator());
-        aw.writer.print("{s}●{s} " ++ fmt ++ "\n", .{ color, ansi.reset } ++ args) catch return;
-        const bytes = aw.written();
-        if (self.spinner.emitAbove(bytes)) return;
-        _ = std.c.write(std.posix.STDERR_FILENO, (bytes).ptr, (bytes).len);
-        return;
+        const bytes = std.fmt.allocPrint(a.allocator(), "{s}●{s} " ++ fmt ++ "\n", .{ color, ansi.reset } ++ args) catch return;
+        return self.emitStderr(bytes);
     }
     std.debug.print("{s}{s}{s}: " ++ fmt ++ "{s}\n", .{ ansi.bold, color, label } ++ args ++ .{ansi.reset});
 }

src/agent/prompt_assist.zig

@@ -569,10 +569,8 @@ fn renderMetaHint(state: *State, meta: *const SlashCommand.MetaCommand, body: []
     }
 
     if (body.len == 0) {
-        var frags: [1][]const u8 = .{meta.hint};
-        return writeHints(if (ends_ws) "" else " ", &frags);
+        return writeHints(if (ends_ws) "" else " ", &.{meta.hint});
     }
-    if (ends_ws) return null;
     if (meta.tag == .load or meta.tag == .save) return ghostPathFirstMatch(body);
     return ghostFirstMatch(meta.values, body, "");
 }

src/agent/settings.zig

@@ -336,9 +336,7 @@ pub fn resolveModelName(opts: Config.Agent, resolved: ?ResolvedProvider, remembe
     if (resolved) |r| {
         // Use the remembered model whenever it matches the chosen provider,
         // not only when the provider itself came from the remembered file.
-        if (remembered) |rem| {
-            if (rem.provider) |p| if (p == r.credential.provider) return rem.model;
-        }
+        if (remembered) |rem| if (rem.provider == r.credential.provider) return rem.model;
         return zenai.provider.defaultModel(r.credential.provider);
     }
     return "";
@@ -378,12 +376,6 @@ pub fn resolveSearchEngine(remembered: ?Remembered) lp.tools.SearchEngine {
     return .auto;
 }
 
-const ReconciledModel = union(enum) {
-    /// Owned by the allocator passed to reconcileModel.
-    use: []u8,
-    abort,
-};
-
 /// Validate `desired` against the provider's catalog, mirroring the interactive
 /// `/model` command. Empty list (unreachable server) leaves it unchecked; an
 /// explicit unlisted model is fatal. The local servers (Ollama, llama.cpp) have
@@ -395,23 +387,23 @@ pub fn reconcileModel(
     desired: []const u8,
     base_url: ?[:0]const u8,
     explicit: bool,
-) !ReconciledModel {
+) ![]u8 {
     // A subscription provider can't list models via the provider API; trust the
     // desired model as-is rather than error against `/models`.
-    if (auth.descriptorFor(credential.provider) != null) return .{ .use = try allocator.dupe(u8, desired) };
+    if (auth.descriptorFor(credential.provider) != null) return try allocator.dupe(u8, desired);
 
     var arena: std.heap.ArenaAllocator = .init(allocator);
     defer arena.deinit();
     const ids: []const []const u8 = zenai.provider.listChatModelIds(lp.io, allocator, arena.allocator(), credential.provider, credential.keySlice(), .{ .base_url = base_url, .environ = lp.environ() }) catch &.{};
-    if (ids.len == 0 or string.isOneOf(desired, ids)) return .{ .use = try allocator.dupe(u8, desired) };
+    if (ids.len == 0 or string.isOneOf(desired, ids)) return try allocator.dupe(u8, desired);
 
     if (!explicit) {
         switch (credential.provider) {
             .ollama, .llama_cpp => {},
-            else => return .{ .use = try allocator.dupe(u8, desired) },
+            else => return try allocator.dupe(u8, desired),
         }
         std.debug.print("Default {s} model '{s}' is not loaded; using '{s}'.\n", .{ @tagName(credential.provider), desired, ids[0] });
-        return .{ .use = try allocator.dupe(u8, ids[0]) };
+        return try allocator.dupe(u8, ids[0]);
     }
 
     if (credential.provider == .ollama) {
@@ -426,7 +418,7 @@ pub fn reconcileModel(
             .{ desired, @tagName(credential.provider) },
         );
     }
-    return .abort;
+    return error.ModelNotAvailable;
 }
 
 const testing = @import("../testing.zig");