mirror of
https://github.com/lightpanda-io/browser.git
synced 2026-06-11 01:25:53 -04:00
refactor: use explicit type names and encapsulate spinner state
- Replace `Self` with `Recorder` and `Verifier` for improved clarity. - Add `Spinner.isEnabled()` to encapsulate atomic state access. - Shorten and refine various comments across the codebase.
This commit is contained in:
@@ -1194,7 +1194,7 @@ fn handleToolCall(ctx: *anyopaque, allocator: std.mem.Allocator, tool_name: []co
|
||||
const self: *Agent = @ptrCast(@alignCast(ctx));
|
||||
// The spinner doesn't render args, and `agentToolDone` skips the body
|
||||
// line at low verbosity — don't pay for the stringify when nobody reads it.
|
||||
const needs_args = self.terminal.spinner.enabled.load(.monotonic) or self.terminal.verbosity != .low;
|
||||
const needs_args = self.terminal.spinner.isEnabled() or self.terminal.verbosity != .low;
|
||||
const args_str: []const u8 = if (needs_args) (if (arguments) |v|
|
||||
std.json.Stringify.valueAlloc(allocator, v, .{}) catch ""
|
||||
else
|
||||
@@ -1374,10 +1374,8 @@ fn pickModel(
|
||||
var arena: std.heap.ArenaAllocator = .init(allocator);
|
||||
defer arena.deinit();
|
||||
|
||||
// Known limitation: this runs before `SigBridge.attach`, so a Ctrl-C
|
||||
// pressed during the model listing is dropped silently. The HTTP call
|
||||
// is synchronous and uninterruptible; press Ctrl-C again at the picker
|
||||
// prompt (which surfaces it as `error.UserCancelled` via stdin EINTR).
|
||||
// Runs before `SigBridge.attach` — Ctrl-C during the synchronous HTTP
|
||||
// fetch is dropped; the picker prompt catches the next press via stdin EINTR.
|
||||
std.debug.print("Fetching models for {s}…\n", .{@tagName(provider)});
|
||||
const ids = zenai.provider.listChatModelIds(allocator, arena.allocator(), provider, api_key, base_url) catch |err| {
|
||||
log.fatal(.app, "list models failed", .{ .err = @errorName(err) });
|
||||
|
||||
@@ -57,9 +57,8 @@ const State = union(enum) {
|
||||
tool: ToolState,
|
||||
};
|
||||
|
||||
/// Atomic so the agent-thread fast-paths (`if (!self.enabled.load(...))`) and
|
||||
/// the under-lock write in `ensureWorkerLocked` don't race if a future caller
|
||||
/// touches the spinner from a non-agent thread.
|
||||
/// Atomic so the unlocked fast-path reads (`isEnabled`) don't race the
|
||||
/// under-lock write in `ensureWorkerLocked`.
|
||||
enabled: std.atomic.Value(bool),
|
||||
|
||||
mu: std.Thread.Mutex = .{},
|
||||
@@ -80,6 +79,10 @@ pub fn init(is_repl: bool, stderr_is_tty: bool) Spinner {
|
||||
return .{ .enabled = .init(is_repl and stderr_is_tty) };
|
||||
}
|
||||
|
||||
pub inline fn isEnabled(self: *const Spinner) bool {
|
||||
return self.enabled.load(.monotonic);
|
||||
}
|
||||
|
||||
pub fn deinit(self: *Spinner) void {
|
||||
if (self.thread) |t| {
|
||||
self.mu.lock();
|
||||
@@ -93,7 +96,7 @@ pub fn deinit(self: *Spinner) void {
|
||||
|
||||
/// Begin a new agent turn. Spawns the worker thread on first call.
|
||||
pub fn start(self: *Spinner) void {
|
||||
if (!self.enabled.load(.monotonic)) return;
|
||||
if (!self.isEnabled()) return;
|
||||
self.mu.lock();
|
||||
defer self.mu.unlock();
|
||||
self.state = .thinking;
|
||||
@@ -119,7 +122,7 @@ fn ensureWorkerLocked(self: *Spinner) void {
|
||||
/// End an agent turn cleanly: clear the indicator, commit a one-line summary,
|
||||
/// reset state. Called from a `defer` in the agent code so it always runs.
|
||||
pub fn stop(self: *Spinner) void {
|
||||
if (!self.enabled.load(.monotonic)) return;
|
||||
if (!self.isEnabled()) return;
|
||||
self.mu.lock();
|
||||
defer self.mu.unlock();
|
||||
if (self.state == .idle) return;
|
||||
@@ -141,7 +144,7 @@ pub fn stop(self: *Spinner) void {
|
||||
/// End a turn with no commit. The caller is responsible for surfacing the
|
||||
/// outcome — tool results, error messages, or summaries.
|
||||
pub fn cancel(self: *Spinner) void {
|
||||
if (!self.enabled.load(.monotonic)) return;
|
||||
if (!self.isEnabled()) return;
|
||||
self.mu.lock();
|
||||
defer self.mu.unlock();
|
||||
if (self.state == .idle) return;
|
||||
@@ -155,7 +158,7 @@ pub fn cancel(self: *Spinner) void {
|
||||
/// without a preceding `start()` (state `.idle`) the label drops the `agent:`
|
||||
/// prefix — that path is for user-typed REPL commands, not LLM tool calls.
|
||||
pub fn setTool(self: *Spinner, name: []const u8, args: []const u8) void {
|
||||
if (!self.enabled.load(.monotonic)) return;
|
||||
if (!self.isEnabled()) return;
|
||||
self.mu.lock();
|
||||
defer self.mu.unlock();
|
||||
const manual = self.state == .idle;
|
||||
@@ -178,7 +181,7 @@ pub fn setTool(self: *Spinner, name: []const u8, args: []const u8) void {
|
||||
/// for the rest of the dwell window (`min_tool_display_ns`), then the
|
||||
/// indicator returns to thinking like any other call.
|
||||
pub fn markToolFailed(self: *Spinner) void {
|
||||
if (!self.enabled.load(.monotonic)) return;
|
||||
if (!self.isEnabled()) return;
|
||||
self.mu.lock();
|
||||
defer self.mu.unlock();
|
||||
switch (self.state) {
|
||||
@@ -194,7 +197,7 @@ pub fn markToolFailed(self: *Spinner) void {
|
||||
/// honors `min_tool_display_ns` — if the current tool label has not been
|
||||
/// up long enough, the flip is deferred until it has.
|
||||
pub fn setThinking(self: *Spinner) void {
|
||||
if (!self.enabled.load(.monotonic)) return;
|
||||
if (!self.isEnabled()) return;
|
||||
self.mu.lock();
|
||||
defer self.mu.unlock();
|
||||
switch (self.state) {
|
||||
@@ -208,7 +211,7 @@ pub fn setThinking(self: *Spinner) void {
|
||||
/// Print `text` (assumed to include its own newline) above the indicator,
|
||||
/// then leave the indicator to repaint itself on the next tick.
|
||||
pub fn emitAbove(self: *Spinner, text: []const u8) bool {
|
||||
if (!self.enabled.load(.monotonic)) return false;
|
||||
if (!self.isEnabled()) return false;
|
||||
self.mu.lock();
|
||||
defer self.mu.unlock();
|
||||
if (self.state == .idle) return false;
|
||||
|
||||
@@ -151,7 +151,7 @@ pub fn endTool(self: *Terminal, ok: bool) void {
|
||||
/// gated on `medium`+. In non-TTY contexts ANSI is still emitted —
|
||||
/// pipes that strip color see plain text via the bullet character.
|
||||
pub fn agentToolDone(self: *Terminal, name: []const u8, args: []const u8, ok: bool) void {
|
||||
const spinner_on = self.spinner.enabled.load(.monotonic);
|
||||
const spinner_on = self.spinner.isEnabled();
|
||||
if (spinner_on and !ok) self.spinner.markToolFailed();
|
||||
if (!atLeast(self.verbosity, .medium)) return;
|
||||
|
||||
|
||||
@@ -22,7 +22,7 @@ const log = lp.log;
|
||||
const testing = @import("../testing.zig");
|
||||
const Command = @import("Command.zig");
|
||||
|
||||
const Self = @This();
|
||||
const Recorder = @This();
|
||||
|
||||
allocator: std.mem.Allocator,
|
||||
/// Open append-mode handle while recording is active. Becomes null when a
|
||||
@@ -38,7 +38,7 @@ lines: u32,
|
||||
buf: std.Io.Writer.Allocating,
|
||||
|
||||
/// Append-open `path`, inserting a leading newline if the file is non-empty.
|
||||
pub fn init(allocator: std.mem.Allocator, path: []const u8) !Self {
|
||||
pub fn init(allocator: std.mem.Allocator, path: []const u8) !Recorder {
|
||||
const owned_path = try allocator.dupe(u8, path);
|
||||
errdefer allocator.free(owned_path);
|
||||
const file = try openForAppend(path);
|
||||
@@ -60,23 +60,23 @@ fn openForAppend(path: []const u8) !std.fs.File {
|
||||
return f;
|
||||
}
|
||||
|
||||
pub fn deinit(self: *Self) void {
|
||||
pub fn deinit(self: *Recorder) void {
|
||||
self.buf.deinit();
|
||||
if (self.file) |f| f.close();
|
||||
self.allocator.free(self.path);
|
||||
}
|
||||
|
||||
pub fn isActive(self: *const Self) bool {
|
||||
pub fn isActive(self: *const Recorder) bool {
|
||||
return self.file != null;
|
||||
}
|
||||
|
||||
pub fn record(self: *Self, cmd: Command.Command) void {
|
||||
pub fn record(self: *Recorder, cmd: Command.Command) void {
|
||||
if (self.file == null) return;
|
||||
if (!cmd.isRecorded()) return;
|
||||
self.tryRecord(cmd) catch |err| self.disable(err);
|
||||
}
|
||||
|
||||
fn tryRecord(self: *Self, cmd: Command.Command) !void {
|
||||
fn tryRecord(self: *Recorder, cmd: Command.Command) !void {
|
||||
self.buf.clearRetainingCapacity();
|
||||
try cmd.format(&self.buf.writer);
|
||||
try self.buf.writer.writeByte('\n');
|
||||
@@ -92,12 +92,12 @@ fn tryRecord(self: *Self, cmd: Command.Command) !void {
|
||||
self.lines += 1;
|
||||
}
|
||||
|
||||
pub fn recordComment(self: *Self, comment: []const u8) void {
|
||||
pub fn recordComment(self: *Recorder, comment: []const u8) void {
|
||||
if (self.file == null) return;
|
||||
self.tryRecordComment(comment) catch |err| self.disable(err);
|
||||
}
|
||||
|
||||
fn tryRecordComment(self: *Self, comment: []const u8) !void {
|
||||
fn tryRecordComment(self: *Recorder, comment: []const u8) !void {
|
||||
self.buf.clearRetainingCapacity();
|
||||
// Embedded newlines would smuggle an executable line into the script on
|
||||
// replay (e.g. `# foo\nGOTO https://attacker`). Emit each line of the
|
||||
@@ -115,10 +115,8 @@ fn tryRecordComment(self: *Self, comment: []const u8) !void {
|
||||
|
||||
/// Any failure along the record path — buffer-write OOM, scrub OOM, or file
|
||||
/// write — flips the recorder to inactive so subsequent calls become silent
|
||||
/// no-ops and `isActive()` reflects the stopped state. Previously a buffer
|
||||
/// failure would `catch return` while leaving the file open and `isActive()`
|
||||
/// true, silently dropping the line.
|
||||
fn disable(self: *Self, err: anyerror) void {
|
||||
/// no-ops and `isActive()` reflects the stopped state.
|
||||
fn disable(self: *Recorder, err: anyerror) void {
|
||||
log.warn(.app, "recording disabled", .{ .err = @errorName(err) });
|
||||
if (self.file) |f| {
|
||||
f.close();
|
||||
@@ -132,7 +130,7 @@ test "record writes state-mutating commands" {
|
||||
|
||||
const file = tmp.dir.createFile("test.lp", .{ .read = true }) catch unreachable;
|
||||
|
||||
var recorder: Self = .{
|
||||
var recorder: Recorder = .{
|
||||
.allocator = std.testing.allocator,
|
||||
.file = file,
|
||||
.path = try std.testing.allocator.dupe(u8, "test.lp"),
|
||||
@@ -180,7 +178,7 @@ test "record skips empty and comment lines" {
|
||||
|
||||
const file = tmp.dir.createFile("test2.lp", .{ .read = true }) catch unreachable;
|
||||
|
||||
var recorder: Self = .{
|
||||
var recorder: Recorder = .{
|
||||
.allocator = std.testing.allocator,
|
||||
.file = file,
|
||||
.path = try std.testing.allocator.dupe(u8, "test.lp"),
|
||||
@@ -208,7 +206,7 @@ test "lines counter tracks successful appends" {
|
||||
|
||||
const file = tmp.dir.createFile("count.lp", .{ .read = true }) catch unreachable;
|
||||
|
||||
var recorder: Self = .{
|
||||
var recorder: Recorder = .{
|
||||
.allocator = std.testing.allocator,
|
||||
.file = file,
|
||||
.path = try std.testing.allocator.dupe(u8, "test.lp"),
|
||||
@@ -240,7 +238,7 @@ test "init appends to an existing file without truncating" {
|
||||
var path_buf: [std.fs.max_path_bytes]u8 = undefined;
|
||||
const abs_path = tmp.dir.realpath("script.lp", &path_buf) catch unreachable;
|
||||
|
||||
var recorder: Self = try .init(std.testing.allocator, abs_path);
|
||||
var recorder: Recorder = try .init(std.testing.allocator, abs_path);
|
||||
defer recorder.deinit();
|
||||
recorder.record(Command.parse("CLICK 'Login'"));
|
||||
|
||||
@@ -267,7 +265,7 @@ test "recordComment splits embedded newlines into separate comment lines" {
|
||||
defer tmp.cleanup();
|
||||
|
||||
const file = tmp.dir.createFile("multi.lp", .{ .read = true }) catch unreachable;
|
||||
var recorder: Self = .{
|
||||
var recorder: Recorder = .{
|
||||
.allocator = std.testing.allocator,
|
||||
.file = file,
|
||||
.path = try std.testing.allocator.dupe(u8, "test.lp"),
|
||||
@@ -302,7 +300,7 @@ test "record disables recorder on write failure" {
|
||||
break :blk tmp.dir.openFile("ro.lp", .{ .mode = .read_only }) catch unreachable;
|
||||
};
|
||||
|
||||
var recorder: Self = .{
|
||||
var recorder: Recorder = .{
|
||||
.allocator = std.testing.allocator,
|
||||
.file = file,
|
||||
.path = try std.testing.allocator.dupe(u8, "test.lp"),
|
||||
@@ -331,7 +329,7 @@ test "init creates the file if missing" {
|
||||
var full_buf: [std.fs.max_path_bytes]u8 = undefined;
|
||||
const abs_path = std.fmt.bufPrint(&full_buf, "{s}/fresh.lp", .{dir_path}) catch unreachable;
|
||||
|
||||
var recorder: Self = try .init(std.testing.allocator, abs_path);
|
||||
var recorder: Recorder = try .init(std.testing.allocator, abs_path);
|
||||
defer recorder.deinit();
|
||||
recorder.record(Command.parse("GOTO https://example.com"));
|
||||
|
||||
|
||||
@@ -22,7 +22,7 @@ const browser_tools = lp.tools;
|
||||
const Command = @import("Command.zig");
|
||||
const CDPNode = @import("../cdp/Node.zig");
|
||||
|
||||
const Self = @This();
|
||||
const Verifier = @This();
|
||||
|
||||
session: *lp.Session,
|
||||
node_registry: *CDPNode.Registry,
|
||||
@@ -33,9 +33,8 @@ pub const VerifyResult = union(enum) {
|
||||
inconclusive,
|
||||
};
|
||||
|
||||
/// Closed set of element properties the verifier can probe. Keeps the JS
|
||||
/// template injection-free (no caller-supplied expression text) and
|
||||
/// guarantees each variant produces a valid `el`-rooted JS expression.
|
||||
/// Closed set of element properties the verifier can probe — keeps the JS
|
||||
/// template injection-free (no caller-supplied expression text).
|
||||
const ElementProperty = enum {
|
||||
value,
|
||||
checked_string,
|
||||
@@ -48,17 +47,14 @@ const ElementProperty = enum {
|
||||
}
|
||||
};
|
||||
|
||||
/// Static fallback used when allocPrint OOMs while formatting the reason —
|
||||
/// keeps `VerifyResult.failed` non-optional so callers don't have to
|
||||
/// distinguish "no reason" from "OOM" (which they uniformly couldn't
|
||||
/// recover from anyway).
|
||||
/// Fallback when allocPrint OOMs — lets `VerifyResult.failed` stay non-optional.
|
||||
const failed_reason_oom = "verification failed (out of memory while formatting reason)";
|
||||
|
||||
/// Verify that a command achieved its intent after execution. Only called
|
||||
/// when the command did not hard-fail (ExecResult.failed == false).
|
||||
/// Commands without a dedicated verifier return `.inconclusive` so callers
|
||||
/// can distinguish "no verification available" from "explicitly verified".
|
||||
pub fn verify(self: *Self, arena: std.mem.Allocator, cmd: Command.Command) VerifyResult {
|
||||
pub fn verify(self: *Verifier, arena: std.mem.Allocator, cmd: Command.Command) VerifyResult {
|
||||
return switch (cmd) {
|
||||
.type_cmd => |args| self.verifyFill(arena, args.selector, args.value),
|
||||
.check => |args| self.verifyCheck(arena, args.selector, args.checked),
|
||||
@@ -67,7 +63,7 @@ pub fn verify(self: *Self, arena: std.mem.Allocator, cmd: Command.Command) Verif
|
||||
};
|
||||
}
|
||||
|
||||
fn verifyFill(self: *Self, arena: std.mem.Allocator, selector: []const u8, expected_value: []const u8) VerifyResult {
|
||||
fn verifyFill(self: *Verifier, arena: std.mem.Allocator, selector: []const u8, expected_value: []const u8) VerifyResult {
|
||||
// Secret env-var references can't be compared literally — just
|
||||
// verify the field isn't empty after substitution.
|
||||
if (std.mem.indexOf(u8, expected_value, "$LP_") != null) {
|
||||
@@ -79,12 +75,12 @@ fn verifyFill(self: *Self, arena: std.mem.Allocator, selector: []const u8, expec
|
||||
return self.verifyElementValue(arena, selector, .{ .property = .value, .expected = expected_value, .label = "value" });
|
||||
}
|
||||
|
||||
fn verifyCheck(self: *Self, arena: std.mem.Allocator, selector: []const u8, expected: bool) VerifyResult {
|
||||
fn verifyCheck(self: *Verifier, arena: std.mem.Allocator, selector: []const u8, expected: bool) VerifyResult {
|
||||
const expected_str: []const u8 = if (expected) "true" else "false";
|
||||
return self.verifyElementValue(arena, selector, .{ .property = .checked_string, .expected = expected_str, .label = "checked state" });
|
||||
}
|
||||
|
||||
fn verifySelect(self: *Self, arena: std.mem.Allocator, selector: []const u8, expected_value: []const u8) VerifyResult {
|
||||
fn verifySelect(self: *Verifier, arena: std.mem.Allocator, selector: []const u8, expected_value: []const u8) VerifyResult {
|
||||
return self.verifyElementValue(arena, selector, .{ .property = .value, .expected = expected_value, .label = "selected value" });
|
||||
}
|
||||
|
||||
@@ -94,7 +90,7 @@ const Check = struct {
|
||||
label: []const u8,
|
||||
};
|
||||
|
||||
fn verifyElementValue(self: *Self, arena: std.mem.Allocator, selector: []const u8, check: Check) VerifyResult {
|
||||
fn verifyElementValue(self: *Verifier, arena: std.mem.Allocator, selector: []const u8, check: Check) VerifyResult {
|
||||
const actual = self.queryElementProperty(arena, selector, check.property) orelse return .inconclusive;
|
||||
if (!std.mem.eql(u8, actual, check.expected)) {
|
||||
const msg = std.fmt.allocPrint(arena, "element {s} is \"{s}\" (expected \"{s}\")", .{ check.label, actual, check.expected }) catch failed_reason_oom;
|
||||
@@ -103,7 +99,7 @@ fn verifyElementValue(self: *Self, arena: std.mem.Allocator, selector: []const u
|
||||
return .passed;
|
||||
}
|
||||
|
||||
fn queryElementProperty(self: *Self, arena: std.mem.Allocator, selector: []const u8, property: ElementProperty) ?[]const u8 {
|
||||
fn queryElementProperty(self: *Verifier, arena: std.mem.Allocator, selector: []const u8, property: ElementProperty) ?[]const u8 {
|
||||
const selector_json = std.json.Stringify.valueAlloc(arena, selector, .{}) catch return null;
|
||||
const script = std.fmt.allocPrint(
|
||||
arena,
|
||||
|
||||
Reference in New Issue
Block a user