lobo/tally
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

human review: units/float_interp

Browse source
This commit is contained in:
Emil Lerch 2026-08-21 15:25:31 -07:00
parent bc225066dc
commit e3e7c61459
Signed by: lobo
GPG key ID: A7B62D657EF764F8
4 changed files with 204 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
.kiro/specs/calculator/design.md
View file

@ -907,9 +907,23 @@ table by canonical name and alias:
1. An **exact, case-sensitive** match is tried first. This is what keeps
case-distinguished units meaningful: `K` is Kelvin (not a "kilo" something),
`B` is byte, and `kB` (1000) never silently becomes `KiB` (1024).
`B` is byte and `b` is bit, `Mb` is megabit and `MB` is megabyte, and `kB`
(1000) never silently becomes `KiB` (1024).
2. Only if nothing matches exactly does a **case-insensitive** pass run, which
is what makes forgiving input like `KM` or `Celsius` work.
is what makes forgiving input like `KM` or `Celsius` work. That pass must find
**exactly one** unit. Where a fold reaches two units that the tables
deliberately distinguish by case, the name is `AmbiguousUnit` rather than a
guess: `Mbps` is megabits per second and `MBps` is megabytes per second, so
`MBPS` is a factor of eight away from one of the two readings with no way to
tell which was meant. Returning the first fold match is how `MBPS` silently
became `Mbps`, and how `mB` became `MB`.
Bits sit alongside bytes in `digital_storage` (`b`, `kb`, `Mb`, `Gb`, `Tb`, `Pb`),
because the category's whole convention is that lowercase `b` is a bit and
uppercase `B` a byte. Before they existed, `Mb` matched nothing exactly, fell
through to the fold, and `100 Mb to MB` answered `100 MB`: a megabit-to-megabyte
question returning an identity. A bit is an eighth of a byte, so every factor in
the category is still exact.
Conversion goes through the category base unit: `to.fromBase(from.toBase(value))`.
Both units must be in the same category, or the result is `IncompatibleUnits`.

43
.kiro/specs/calculator/tasks.md
View file

@ -914,8 +914,51 @@ plans a `--raw` flag, but today it is exercised only by tests.
100% line coverage, engine 99.44%. CLI output byte-identical across all five rows,
both byte orders, ASCII packing and the multi-base standard-mode view.
### Task 5.22: Bits are not bytes, and a fold that could mean two units says so
Two findings from the `units.zig` review, one a silent wrong answer.
**The prefixed bit units did not exist.** `digital_storage` had `bit` and the byte
ladder, but no `kb`/`Mb`/`Gb`, so `Mb` matched nothing exactly, fell through to the
case-insensitive pass, and resolved to `MB`:
```
100 Mb to MB -> 100 MB = 100 MB now: 100 Mb = 12.5 MB
1 kb to B -> 1 kB = 1,000 B now: 1 kb = 125 B
```
A megabit-to-megabyte question came back as an identity, and the echoed input said
`MB` where the user had written `Mb`. `findUnit`'s doc comment acknowledged the class
of problem and told the reader that "a user who means megabits has to write `Mbit`",
which was not true: no such unit existed. `b`, `kb`, `Mb`, `Gb`, `Tb` and `Pb` are now
in the table with their long forms, and since a bit is an eighth of a byte every
factor stays exact. The binary bit forms (`Kib`, `Mib`) are still absent, which is
deliberate: nothing asked for them and they are rarely written.
**A fold onto two units is an error, not a guess.** The case-insensitive pass
returned the first match and stopped, so a spelling that folded onto two units the
tables distinguish by case got whichever came first in the file. In `data_rate` that
is an eightfold error:
```
100 MBPS to Mbps -> 100 Mbps = 100 Mbps now: error: ambiguous unit name
100 mB to MB -> 100 MB = 100 MB now: error: ambiguous unit name
```
`resolve` now returns `unit`, `unknown` or `ambiguous`, and the pass has to find
exactly one unit; two aliases of one unit are not a conflict, two units are. The new
`AmbiguousUnit` error carries "ambiguous unit name: case tells these units apart".
`findUnit` keeps its `?UnitDef` signature for the internal scans and returns null for
both failures, while `convert` and `parseRequest` report which. `splitTrailingUnit`
returns the error rather than skipping the candidate, because skipping would go
looking for a shorter suffix and answer a different question with no error at all.
The forgiving input the fold exists for is untouched: `100 KM to mi`,
`32 F to celsius`, `1 KIB to B`, `5 in in cm` and `100 mm in in` all still resolve.
### Task 5.21: One table of built-in functions
`evalFunction` dispatched through a chain of `if (mem.eql(u8, name, ...))` blocks
grouped by argument count, spilling into `evalSingleArgFn` and `evalFinancialFn`,
both of which returned `Error!?f64` where null meant "not my name". Four consequences:

1
engine/src/engine.zig
View file

@ -100,6 +100,7 @@ pub fn phrase(err: Error) []const u8 {
// Units
error.UnknownUnit => "unknown unit",
error.AmbiguousUnit => "ambiguous unit name: case tells these units apart",
error.IncompatibleUnits => "incompatible units (different categories)",
// Financial

179
engine/src/units.zig
View file

@ -28,6 +28,9 @@ const std = @import("std");
/// the evaluator had.
pub const Error = error{
UnknownUnit,
/// A name that matches no unit exactly and folds onto more than one. Honouring
/// it would be a guess between units the tables distinguish by case.
AmbiguousUnit,
IncompatibleUnits,
OutOfMemory,
} || number_mod.Error;
@ -262,9 +265,21 @@ const time_units = define(.time, &.{
.{ .name = "yr", .aliases = &.{ "year", "years" }, .factor = "31557600" },
});
// Base: byte. Decimal (kB) and binary (KiB) prefixes are both provided.
// Base: byte. Decimal (kB) and binary (KiB) prefixes are both provided, and bits
// alongside bytes: lowercase `b` is a bit and uppercase `B` a byte, which is the
// convention the whole category depends on. Before the prefixed bit units existed,
// `Mb` matched no unit exactly and fell through to the case-insensitive pass, so
// `100 Mb to MB` answered "100 MB" and a megabit-to-megabyte question came back as
// an identity.
//
// A bit is an eighth of a byte, so every factor here is exact.
const digital_storage_units = define(.digital_storage, &.{
.{ .name = "bit", .aliases = &.{"bits"}, .factor = "0.125" },
.{ .name = "b", .aliases = &.{ "bit", "bits" }, .factor = "0.125" },
.{ .name = "kb", .aliases = &.{ "kilobit", "kilobits" }, .factor = "125" },
.{ .name = "Mb", .aliases = &.{ "megabit", "megabits" }, .factor = "1.25e5" },
.{ .name = "Gb", .aliases = &.{ "gigabit", "gigabits" }, .factor = "1.25e8" },
.{ .name = "Tb", .aliases = &.{ "terabit", "terabits" }, .factor = "1.25e11" },
.{ .name = "Pb", .aliases = &.{ "petabit", "petabits" }, .factor = "1.25e14" },
.{ .name = "B", .aliases = &.{ "byte", "bytes" }, .factor = "1" },
.{ .name = "kB", .aliases = &.{ "kilobyte", "kilobytes" }, .factor = "1e3" },
.{ .name = "MB", .aliases = &.{ "megabyte", "megabytes" }, .factor = "1e6" },
@ -418,32 +433,72 @@ fn matchesIgnoreCase(unit: UnitDef, name: []const u8) bool {
return false;
}
/// What a name resolved to.
const Resolution = union(enum) {
unit: UnitDef,
/// No unit has this name or alias, in any case.
unknown,
/// The name matches nothing exactly and folds onto more than one unit. Picking
/// whichever comes first in the tables answers a different question: `Mbps` is
/// megabits per second and `MBps` is megabytes per second, so `MBPS` is a factor
/// of eight away from one of the two readings and there is no way to tell which
/// was meant.
ambiguous,
};
/// Resolve a name to at most one unit.
///
/// An exact (case-sensitive) match is tried first so that case-distinguished units
/// keep their meaning: "K" stays Kelvin, "B" stays byte, "Mb" stays megabit, and "kB"
/// does not become "KiB".
///
/// Only if nothing matches exactly does a case-insensitive pass run, which is what
/// makes forgiving input like "KM" or "Celsius" work. That pass has to find exactly
/// one unit. It used to return the first fold match and stop, which is how "MBPS"
/// silently became "Mbps".
fn resolve(name: []const u8) Resolution {
if (name.len == 0) return .unknown;
for (categories) |table| {
for (table) |unit| {
if (matchesExact(unit, name)) return .{ .unit = unit };
}
}
var found: ?UnitDef = null;
for (categories) |table| {
for (table) |unit| {
if (!matchesIgnoreCase(unit, name)) continue;
if (found) |first| {
// Two aliases of one unit are not a conflict; two units are.
if (!std.mem.eql(u8, first.name, unit.name)) return .ambiguous;
} else {
found = unit;
}
}
}
if (found) |unit| return .{ .unit = unit };
return .unknown;
}
/// Look up a unit by canonical name or alias.
///
/// An exact (case-sensitive) match is tried first so that case-distinguished
/// units keep their meaning: "K" stays Kelvin, "B" stays byte, and "kB" does not
/// become "KiB". Only if nothing matches exactly does a case-insensitive pass run,
/// which is what makes forgiving input like "KM" or "Celsius" work.
///
/// That fallback is deliberately permissive, so a spelling with no exact entry
/// resolves to whatever differs only in case: "mB" and "Mb" both reach "MB". A
/// user who means megabits has to write "Mbit" or "Mb" ... which is the same
/// string, so they have to write "Mbit". Tightening this would mean rejecting
/// case-variant input outright, which is a bigger behavioural decision than a
/// comment can carry.
/// Null covers both "no such unit" and "several, differing only in case". Callers
/// that report to a user go through `resolveOrError` instead, so the two come out as
/// different messages.
pub fn findUnit(name: []const u8) ?UnitDef {
if (name.len == 0) return null;
for (categories) |table| {
for (table) |unit| {
if (matchesExact(unit, name)) return unit;
}
}
for (categories) |table| {
for (table) |unit| {
if (matchesIgnoreCase(unit, name)) return unit;
}
}
return null;
return switch (resolve(name)) {
.unit => |unit| unit,
.unknown, .ambiguous => null,
};
}
fn resolveOrError(name: []const u8) Error!UnitDef {
return switch (resolve(name)) {
.unit => |unit| unit,
.unknown => Error.UnknownUnit,
.ambiguous => Error.AmbiguousUnit,
};
}
// -- Conversion --
@ -460,8 +515,8 @@ pub fn convertUnits(value: f64, from: UnitDef, to: UnitDef) Error!f64 {
/// Returns UnknownUnit if either name is unrecognized, or IncompatibleUnits if
/// the units belong to different categories.
pub fn convert(value: f64, from_name: []const u8, to_name: []const u8) Error!ConvertResult {
const from = findUnit(from_name) orelse return Error.UnknownUnit;
const to = findUnit(to_name) orelse return Error.UnknownUnit;
const from = try resolveOrError(from_name);
const to = try resolveOrError(to_name);
const result = try convertUnits(value, from, to);
// A single scaling factor only describes the relationship when neither
@ -548,7 +603,7 @@ fn collectSeparators(text: []const u8, out: *[max_separators]Span) usize {
/// A candidate must begin at a token boundary: the preceding character may not
/// be a letter. Without that rule the scan finds units buried inside words, e.g.
/// taking the trailing "s" of "100 smoots" as seconds.
fn splitTrailingUnit(text: []const u8) ?struct { value_text: []const u8, unit: UnitDef } {
fn splitTrailingUnit(text: []const u8) Error!?struct { value_text: []const u8, unit: UnitDef } {
var i: usize = 0;
while (i < text.len) : (i += 1) {
// The candidate must start exactly at i, so skip whitespace positions.
@ -557,7 +612,15 @@ fn splitTrailingUnit(text: []const u8) ?struct { value_text: []const u8, unit: U
const candidate = std.mem.trim(u8, text[i..], whitespace);
if (candidate.len == 0) break;
const unit = findUnit(candidate) orelse continue;
const unit = switch (resolve(candidate)) {
.unit => |found| found,
.unknown => continue,
// A candidate sitting at a token boundary that folds onto several units
// is the unit the user meant, spelled in a way that does not say which.
// Reported rather than skipped, because skipping it would go looking for
// a shorter suffix and answer some other question entirely.
.ambiguous => return Error.AmbiguousUnit,
};
const value_text = std.mem.trim(u8, text[0..i], whitespace);
// A unit with nothing in front of it is not a conversion we can do.
@ -598,11 +661,18 @@ pub fn parseRequest(text: []const u8) Error!?ConversionRequest {
const right = std.mem.trim(u8, text[sep.end..], whitespace);
if (left.len == 0 or right.len == 0) continue;
const to_unit = findUnit(right) orelse {
failure = Error.UnknownUnit;
const to_unit = blk: {
switch (resolve(right)) {
.unit => |unit| break :blk unit,
.unknown => failure = Error.UnknownUnit,
.ambiguous => failure = Error.AmbiguousUnit,
}
continue;
};
const split = splitTrailingUnit(left) orelse {
const split = (splitTrailingUnit(left) catch |err| {
failure = err;
continue;
}) orelse {
failure = Error.UnknownUnit;
continue;
};
@ -952,9 +1022,48 @@ test "exact match wins over case-insensitive match" {
const k = findUnit("K").?;
try testing.expectEqual(UnitCategory.temperature, k.category);
try testing.expectEqualStrings("K", k.name);
// Likewise "B" is byte, not "b"
const b = findUnit("B").?;
try testing.expectEqualStrings("B", b.name);
// Likewise "B" is byte and "b" is bit, each by exact match.
try testing.expectEqualStrings("B", findUnit("B").?.name);
try testing.expectEqualStrings("b", findUnit("b").?.name);
try testing.expectEqualStrings("Mb", findUnit("Mb").?.name);
try testing.expectEqualStrings("MB", findUnit("MB").?.name);
}
// -- Bits are not bytes, and case is how the tables say so --
test "prefixed bit units convert exactly against their byte counterparts" {
// `100 Mb to MB` used to answer "100 MB": there were no prefixed bit units, so
// "Mb" matched nothing exactly and the case-insensitive pass handed back "MB".
try expectConvert(12.5, 100, "Mb", "MB", 1e-12);
try expectConvert(125, 1, "kb", "B", 1e-12);
try expectConvert(1, 8, "b", "B", 1e-12);
try expectConvert(125, 1, "Gb", "MB", 1e-12);
try expectConvert(8, 1, "B", "b", 1e-12);
// The long forms too, which are unambiguous whatever the case.
try expectConvert(12.5, 100, "megabits", "megabytes", 1e-12);
}
test "a name that folds onto two units is ambiguous, not a guess" {
// `Mbps` is megabits per second and `MBps` is megabytes per second, so "MBPS"
// is a factor of eight from one of them and there is no way to tell which. It
// used to resolve to whichever appeared first in the table.
try testing.expectError(Error.AmbiguousUnit, convert(100, "MBPS", "Mbps"));
try testing.expectError(Error.AmbiguousUnit, convert(100, "mB", "MB"));
try testing.expectError(Error.AmbiguousUnit, convert(100, "KB", "B"));
try testing.expect(findUnit("MBPS") == null);
try testing.expect(findUnit("mB") == null);
// An exact spelling of either is still fine, and so is a fold that reaches
// exactly one unit.
try expectConvert(12.5, 100, "Mbps", "MBps", 1e-12);
try expectConvert(1000, 1, "KM", "M", 1e-9);
}
test "an ambiguous unit in a conversion request reports itself" {
// Rather than skipping the candidate and resolving some shorter suffix, which
// would answer a different question with no error at all.
try testing.expectError(Error.AmbiguousUnit, parseRequest("100 MBPS to Mbps"));
try testing.expectError(Error.AmbiguousUnit, parseRequest("100 mB to MB"));
}
test "findUnit returns null for unknown names" {