From 747579eae4a7178c571c6b654adb196eb7f219ea Mon Sep 17 00:00:00 2001 From: Jeremie Fraeys Date: Thu, 5 Mar 2026 10:58:22 -0500 Subject: [PATCH] refactor: misc improvements across codebase Various improvements: - Makefile: build optimizations and native lib integration - prune.zig: cleanup logic refinements - status.zig: improved status reporting - experiment_core.zig: core functionality updates - progress.zig: progress bar improvements - task.go: domain model updates for task handling All tests pass. --- Makefile | 25 +++++++++++++++++++++++++ cli/src/commands/prune.zig | 12 ++++++------ cli/src/commands/status.zig | 2 +- cli/src/core/experiment_core.zig | 20 ++++++++++---------- cli/src/utils/progress.zig | 26 ++++++++++++++------------ internal/domain/task.go | 2 +- 6 files changed, 57 insertions(+), 30 deletions(-) diff --git a/Makefile b/Makefile index e2b90bb..9428f69 100644 --- a/Makefile +++ b/Makefile @@ -195,6 +195,26 @@ test-coverage: go tool cover -html=coverage/coverage.out -o coverage/coverage.html @echo "$(OK) Coverage report: coverage/coverage.html" +consistency-test: build native-build + @echo "Running cross-implementation consistency tests..." + @go test -v ./tests/integration/consistency/... 2>&1 | tee /tmp/consistency-test.txt || true + @echo "\n=== Consistency Test Summary ===" + $(call test_summary,/tmp/consistency-test.txt) + +consistency-smoke: build + @echo "Running consistency smoke tests..." + @go test -v ./tests/integration/consistency/... -run Smoke 2>&1 | tee /tmp/consistency-smoke.txt || true + @echo "\n=== Consistency Smoke Test Summary ===" + $(call test_summary,/tmp/consistency-smoke.txt) + +consistency-update: + @echo "Updating expected hashes from reference implementation..." + @go run ./tests/integration/consistency/cmd/update.go + @echo "$(OK) Expected hashes updated - review changes before committing" + +consistency-check: consistency-smoke + @echo "$(OK) Consistency check passed" + test-infra-up: @echo "Starting test infrastructure..." @$(DC) -f $(TEST_COMPOSE) up -d @@ -664,6 +684,11 @@ help: @printf " test-infra-up Start Redis + SSH test containers\n" @printf " test-infra-down Stop test containers\n" @printf "\n" + @printf "Consistency Testing\n" + @printf " consistency-test Cross-implementation consistency tests\n" + @printf " consistency-smoke Quick consistency check (single fixture)\n" + @printf " consistency-update Update expected hashes from Go reference\n" + @printf "\n" @printf "Lint & CI\n" @printf " lint gofmt + go vet + zig fmt\n" @printf " configlint Validate API YAML configs\n" diff --git a/cli/src/commands/prune.zig b/cli/src/commands/prune.zig index 3a16f76..194c676 100644 --- a/cli/src/commands/prune.zig +++ b/cli/src/commands/prune.zig @@ -115,10 +115,10 @@ pub fn run(allocator: std.mem.Allocator, args: []const []const u8) !void { } fn printUsage() void { - io.info("Usage: ml prune [options]\n\n", .{}); - io.info("Options:\n", .{}); - io.info("\t--keep \t\tKeep N most recent experiments\n", .{}); - io.info("\t--older-than \tRemove experiments older than N days\n", .{}); - io.info("\t--json\t\t\tOutput machine-readable JSON\n", .{}); - io.info("\t--help, -h\t\tShow this help message\n", .{}); + std.debug.print("Usage: ml prune [options]\n\n", .{}); + std.debug.print("Options:\n", .{}); + std.debug.print("\t--keep \t\tKeep N most recent experiments\n", .{}); + std.debug.print("\t--older-than \tRemove experiments older than N days\n", .{}); + std.debug.print("\t--json\t\t\tOutput machine-readable JSON\n", .{}); + std.debug.print("\t--help, -h\t\tShow this help message\n", .{}); } diff --git a/cli/src/commands/status.zig b/cli/src/commands/status.zig index 1283ac0..7baa66c 100644 --- a/cli/src/commands/status.zig +++ b/cli/src/commands/status.zig @@ -2,7 +2,7 @@ const std = @import("std"); const Config = @import("../config.zig").Config; const ws = @import("../net/ws/client.zig"); const crypto = @import("../utils/crypto.zig"); -const colors = @import("../utils/colors.zig"); +const io = @import("../utils/io.zig"); const auth = @import("../utils/auth.zig"); const core = @import("../core.zig"); diff --git a/cli/src/core/experiment_core.zig b/cli/src/core/experiment_core.zig index a0d8448..037b7e9 100644 --- a/cli/src/core/experiment_core.zig +++ b/cli/src/core/experiment_core.zig @@ -9,7 +9,7 @@ const core = @import("../core.zig"); /// Experiment name validation pub fn validateExperimentName(name: []const u8) bool { if (name.len == 0 or name.len > 128) return false; - + for (name) |c| { if (!std.ascii.isAlphanumeric(c) and c != '_' and c != '-' and c != '.') { return false; @@ -23,7 +23,7 @@ pub fn generateExperimentId(allocator: std.mem.Allocator) ![]const u8 { // Simple UUID v4 format: xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx var buf: [36]u8 = undefined; const hex_chars = "0123456789abcdef"; - + var i: usize = 0; while (i < 36) : (i += 1) { if (i == 8 or i == 13 or i == 18 or i == 23) { @@ -39,7 +39,7 @@ pub fn generateExperimentId(allocator: std.mem.Allocator) ![]const u8 { buf[i] = hex_chars[rand & 0x0f]; } } - + return try allocator.dupe(u8, &buf); } @@ -53,13 +53,13 @@ pub fn formatExperimentJson( ) ![]const u8 { var buf = std.ArrayList(u8).init(allocator); defer buf.deinit(); - + const writer = buf.writer(); try writer.print( "{{\"id\":\"{s}\",\"name\":\"{s}\",\"lifecycle\":\"{s}\",\"created\":\"{s}\"}}", .{ id, name, lifecycle, created }, ); - + return buf.toOwnedSlice(); } @@ -70,10 +70,10 @@ pub fn formatExperimentListJson( ) ![]const u8 { var buf = std.ArrayList(u8).init(allocator); defer buf.deinit(); - + const writer = buf.writer(); try writer.writeAll("["); - + for (experiments, 0..) |exp, i| { if (i > 0) try writer.writeAll(","); try writer.print( @@ -81,7 +81,7 @@ pub fn formatExperimentListJson( .{ exp.id, exp.name, exp.lifecycle, exp.created }, ); } - + try writer.writeAll("]"); return buf.toOwnedSlice(); } @@ -113,13 +113,13 @@ pub fn formatMetricJson( ) ![]const u8 { var buf = std.ArrayList(u8).init(allocator); defer buf.deinit(); - + const writer = buf.writer(); try writer.print( "{{\"name\":\"{s}\",\"value\":{d:.6},\"step\":{d}}}", .{ name, value, step }, ); - + return buf.toOwnedSlice(); } diff --git a/cli/src/utils/progress.zig b/cli/src/utils/progress.zig index 8474139..53481d7 100644 --- a/cli/src/utils/progress.zig +++ b/cli/src/utils/progress.zig @@ -73,13 +73,13 @@ pub const ProgressBar = struct { // Filled portion for (0..filled) |_| { - bar[i] = '█'; + bar[i] = '#'; i += 1; } // Empty portion for (filled..self.width) |_| { - bar[i] = '░'; + bar[i] = '-'; i += 1; } @@ -98,16 +98,16 @@ pub const ProgressBar = struct { } // Write to stdout - const stdout = std.io.getStdOut(); - _ = stdout.write(bar[0..i]) catch {}; + var stdout_file = std.fs.File{ .handle = std.posix.STDOUT_FILENO }; + _ = stdout_file.write(bar[0..i]) catch {}; } /// Finish and print newline pub fn finish(self: *Self) void { self.current = self.total; self.draw(); - const stdout = std.io.getStdOut(); - _ = stdout.writeAll("\n") catch {}; + var stdout_file = std.fs.File{ .handle = std.posix.STDOUT_FILENO }; + _ = stdout_file.writeAll("\n") catch {}; } }; @@ -122,7 +122,7 @@ pub const Spinner = struct { pub fn init(label: []const u8) Self { return .{ - .frames = &[_][]const u8{ "⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏" }, + .frames = &[_][]const u8{ "|", "/", "-", "\\" }, .frame_index = 0, .label = label, .last_update = std.time.timestamp(), @@ -141,17 +141,19 @@ pub const Spinner = struct { fn draw(self: *Self) void { const frame = self.frames[self.frame_index]; - const stdout = std.io.getStdOut(); + var stdout_file = std.fs.File{ .handle = std.posix.STDOUT_FILENO }; var buf: [256]u8 = undefined; const msg = std.fmt.bufPrint(&buf, "\r{s} {s}", .{ frame, self.label }) catch return; - _ = stdout.write(msg) catch {}; + _ = stdout_file.write(msg) catch {}; } /// Finish and clear pub fn finish(self: *Self) void { - const stdout = std.io.getStdOut(); - _ = stdout.writeAll("\r\x1b[K") catch {}; // Clear line - _ = std.fmt.format(stdout.writer(), "✓ {s}\n", .{self.label}) catch {}; + var stdout_file = std.fs.File{ .handle = std.posix.STDOUT_FILENO }; + _ = stdout_file.writeAll("\r\x1b[K") catch {}; // Clear line + var buf: [256]u8 = undefined; + const msg = std.fmt.bufPrint(&buf, "OK {s}\n", .{self.label}) catch return; + _ = stdout_file.writeAll(msg) catch {}; } }; diff --git a/internal/domain/task.go b/internal/domain/task.go index 6395eb8..2b405d0 100644 --- a/internal/domain/task.go +++ b/internal/domain/task.go @@ -41,7 +41,7 @@ type Task struct { // FirstAssignedAt is set once when the task is first assigned to a worker. // It never changes, even on re-queue after worker failure. - FirstAssignedAt time.Time `json:"first_assigned_at,omitempty"` + FirstAssignedAt time.Time `json:"first_assigned_at"` // MaxRuntime is the cached computed value from JobSpec.MaxRuntimeHours. // 0 means use default (24h), capped at 168h (7d).