Skip to content
zit Artifact [23a6d989b0]
Login

Artifact [23a6d989b0]

Check-ins Using Download Hex

Artifact 23a6d989b029982afa93fb6599ef01775f9ff9b09b69f2b2aed9f9151eb63754:

//! Structural pins over which repository representation each read verb opens.
//!
//! `Repository.OpenOptions.mode` defaults to `.full` — a whole-store derive.
//! That default is correct for a write verb and wrong for nearly every reader,
//! and the difference is INVISIBLE: a verb on the wrong side produces exactly
//! the right answer, merely proportional to the store instead of to the work.
//! Nothing fails. No test goes red. `diff` lived that way for as long as it
//! has taken `--from`/`--to`: 0.35s against 0.04s for a one-line diff on a
//! 302MB store, and it surfaced only when someone timed a page built on it.
//!
//! A timing assertion would be the direct test and would also be a flaky one —
//! the thing being pinned is a code-shape fact, so it is pinned as one, the way
//! src/ci_test.zig pins the workflow: embed the source, assert on its text.
//!
//! These are text assertions, not a Zig parse. A pinned call sitting inside a
//! comment would satisfy them. That trade is taken knowingly and is the same
//! one ci_test.zig takes; what the pins buy is the negative space — the
//! reviewed list of verbs that are DELIBERATELY `.full`, which reading alone
//! does not catch.
//!
//! Coverage is the two verb sources embedded below, and is deliberately not
//! claimed to be exhaustive. `bisect` lives in cmd/checkout.zig and is not
//! pinned here. `grep` IS in range and is NOT pinned: it opens `.full` with no
//! recorded reason and reads `checkinsByMtime`, `files` and `logStore` -- the
//! image's own surface -- so it looks like the bug again, but it also walks every
//! check-in's file set, which likely dwarfs a derive, and it wires
//! `--no-cache`, which has to move with the mode (see `crosslink`, which
//! passes `!bypass`). Pinning a value nobody has reasoned about would read as
//! coverage; leaving it named here reads as the open question it is.

const std = @import("std");

const history_source = @embedFile("cmd_history_source");
const tracking_source = @embedFile("cmd_tracking_source");

/// One verb's `openRepo` call site, as it appears in the source.
///
/// The `fast_read` argument is positional (the 8th), so this matches the
/// prefix through `repo_override` and reads the two booleans that follow. A
/// call written differently — a named struct, a wrapper — does not match, and
/// `expectMode` fails rather than silently passing, which is the behavior
/// wanted: an unrecognized call site is an unreviewed one.
fn modeOf(source: []const u8, verb: []const u8, buf: []u8) ?bool {
    const needle = std.fmt.bufPrint(buf, "openRepo(gpa, io, arena, streams, \"{s}\", ", .{verb}) catch return null;
    const at = std.mem.indexOf(u8, source, needle) orelse return null;
    const rest = source[at + needle.len ..];
    const end = std.mem.indexOfScalar(u8, rest, ')') orelse return null;
    var it = std.mem.splitSequence(u8, rest[0..end], ", ");
    _ = it.next() orelse return null; // repo_override
    _ = it.next() orelse return null; // bypass_cache
    const fast_read = it.next() orelse return null;
    if (std.mem.eql(u8, fast_read, "true")) return true;
    if (std.mem.eql(u8, fast_read, "false")) return false;
    return null; // a computed argument is not a pinned one
}

fn expectMode(source: []const u8, verb: []const u8, want_fast_read: bool) !void {
    var buf: [128]u8 = undefined;
    const got = modeOf(source, verb, &buf) orelse {
        std.debug.print(
            "\nreadpath pin: no recognizable openRepo call site for '{s}'\n",
            .{verb},
        );
        return error.ReadPathCallSiteMissing;
    };
    if (got == want_fast_read) return;
    std.debug.print(
        "\nreadpath pin: '{s}' opens fast_read={}, expected {}\n" ++
            "A read verb on `.full` re-derives the whole store per invocation.\n" ++
            "If this change is deliberate, say WHY here and in the call site.\n",
        .{ verb, got, want_fast_read },
    );
    return error.ReadPathModeChanged;
}

test "readpath: the check-in read verbs take the zero-parse image path" {
    // Each of these resolves names and reads trees or content, and needs
    // nothing the image does not carry.
    try expectMode(history_source, "ls", true);
    try expectMode(history_source, "cat", true);
    try expectMode(history_source, "info", true);
    try expectMode(history_source, "leaves", true);
    try expectMode(history_source, "branch", true);
    try expectMode(history_source, "frames", true);

    // The original case. `diff --from/--to` needs two resolved names and two file
    // lists -- exactly `ls`'s surface. It reached past `main.openRepo` to
    // `Repository.open` directly and inherited `.full` by omission.
    try expectMode(tracking_source, "diff", true);

    // Same class, found with it: `artifact` reads only the content log
    // (`resolveArtifactPrefix` and `blob` both go through `logStore`), so the
    // derived view it paid to build was never read.
    try expectMode(history_source, "artifact", true);

    // Argued separately: the image has carried adjacency (the §3.2 merge mark)
    // since layout 2 and the comment and login since layout 6, so the `.full`
    // open timeline kept from before those columns paid a whole-store replay
    // for data it already had. The equivalence e2e pins the output.
    try expectMode(history_source, "timeline", true);
}

test "readpath: the deliberate full-derive readers keep their recorded reason" {
    // The reviewed exception: `.full` because it reads what the image does
    // not carry -- NOT by omission -- and says so at the
    // call site. Pinned together with the comment that justifies it, because
    // an unexplained `.full` is indistinguishable from the bug: flipping one
    // for speed should have to be an argued change (timeline's was),
    // and keeping one should have to stay explained.
    try expectMode(history_source, "annotate", false); // walks §3.3 ancestry
    for ([_][]const u8{
        "A cold read: it walks ancestry", // annotate
    }) |reason| {
        if (std.mem.indexOf(u8, history_source, reason) != null) continue;
        std.debug.print(
            "\nreadpath pin: a `.full` open lost the comment saying why (\"{s}\").\n" ++
                "An unexplained `.full` is indistinguishable from the bug.\n",
            .{reason},
        );
        return error.ReadPathReasonMissing;
    }
}