commit b19c14be45b47b618803c6ad262e6955ca320ab4
parent 27ccb4ac697098313c6620adf4a04cb5bf619736
Author: Jack Mordaunt <jackmordaunt.dev@gmail.com>
Date: Wed, 23 Sep 2026 20:38:21 -0300
odin: ask the model jobs and verify what they report
job is the five readings with their criteria compiled in, each rendering
the part of the change it needs, split by file past the packet cap.
provider asks claude, pi, the console API through a strict tool over
jfm:http, or any command the environment names, and probes the default
chain. cache replays answers from the file the Go tool keeps, keyed the
same way. reviewer asks the jobs at once, puts each job's findings back
to the provider once against the same evidence, retracts what the second
reading does not hold, and fails open where a verdict cannot be asked.
Every prompt is worded as the Go tool words it, so the two tools share
one cache. The driver takes the Go tool's flags, chooses the provider,
and reports through the full contract with snippets, usage and failures.
Run with a stub provider on a repository holding Python, TypeScript and
Rust, both tools report the same 16 findings, id for id, with the same
skipped and failed lists.
Diffstat:
26 files changed, 3039 insertions(+), 112 deletions(-)
diff --git a/justfile b/justfile
@@ -41,7 +41,7 @@ odin-test:
#!/usr/bin/env bash
set -euo pipefail
mkdir -p build
- for p in frontend git tree change finding report check analyser; do
+ for p in txt frontend git tree change finding report check analyser job provider cache reviewer; do
{{odin}} test odin/$p {{odin_flags}} -out:build/${p}_test
done
diff --git a/odin/analyser/analyser.odin b/odin/analyser/analyser.odin
@@ -325,16 +325,6 @@ split_json_objects :: proc(out: string, allocator := context.allocator) -> []str
return chunks[:]
}
-// object is the outermost JSON object in a text that has prose around it.
-object :: proc(text: string) -> (string, bool) {
- start := strings.index_byte(text, '{')
- end := strings.last_index_byte(text, '}')
- if start < 0 || end < start {
- return "", false
- }
- return text[start:end + 1], true
-}
-
// lines_of is a text's lines, for the tools that write one JSON object
// per line.
lines_of :: proc(text: string, allocator := context.temp_allocator) -> []string {
diff --git a/odin/analyser/odin.odin b/odin/analyser/odin.odin
@@ -4,6 +4,8 @@ import "core:encoding/json"
import "core:os"
import "core:strings"
+import "../txt"
+
// odin_check type-checks each package the change touched with every vet
// switch and the compiler's own style, and reads its JSON errors. The
// compiler writes its JSON to stderr and exits non-zero when it has
@@ -83,7 +85,7 @@ Odin_Report :: struct {
// object is taken from wherever it sits.
parse_odin :: proc(tree_dir, out: string, allocator := context.allocator) -> []Diagnostic {
found := make([dynamic]Diagnostic, allocator)
- raw, ok := object(out)
+ raw, ok := txt.object(out)
if !ok {
return found[:]
}
diff --git a/odin/cache/cache.odin b/odin/cache/cache.odin
@@ -0,0 +1,181 @@
+/*
+Package cache remembers what a provider answered, so that a re-run of a
+review whose parts did not change does not ask again. A loop iterates by
+re-running: one fix, one more reading. Without the cache every re-run
+re-asks every job, at full price and with a fresh roll of the dice — the
+same question answered two ways is the flicker that makes a loop chase
+ghosts. With it, a job whose rendered subject is byte for byte the same
+replays the recorded answer, which is both free and stable.
+*/
+package cache
+
+import "core:crypto/sha2"
+import "core:encoding/hex"
+import "core:encoding/json"
+import "core:os"
+import "core:path/filepath"
+import "core:slice"
+import "core:strings"
+import "core:sync"
+import "core:time"
+
+import "../provider"
+
+// version is part of every key, so that a change to what an ask holds
+// invalidates the recorded answers instead of replaying stale ones.
+version :: "review-answers-v1"
+
+// bound is the entry count past which the oldest answers are dropped.
+bound :: 4000
+
+// Entry is one recorded answer and what it cost when it was asked.
+Entry :: struct {
+ text: string `json:"text"`,
+ tokens_in: int `json:"in"`,
+ tokens_out: int `json:"out"`,
+ cached: int `json:"cached"`,
+ cost: f64 `json:"cost"`,
+ at: i64 `json:"at"`,
+}
+
+// Cache is one file of recorded answers, keyed by the exact question.
+// fresh skips the reads but keeps the writes, so --fresh re-asks
+// everything and leaves the answers behind it.
+Cache :: struct {
+ path: string,
+ fresh: bool,
+ entries: map[string]Entry,
+ dirty: bool,
+ hits: int,
+ lock: sync.Mutex,
+}
+
+// open reads the answer file from the user's cache directory, or the path
+// given. A cache that cannot be read is no fault of the review: the run
+// asks the provider as it would have. A corrupt file is answered with
+// nothing; the next save rewrites it.
+open :: proc(fresh: bool, path := "", allocator := context.allocator) -> (c: ^Cache, ok: bool) {
+ location := path
+ if location == "" {
+ dir, err := os.user_cache_dir(context.temp_allocator)
+ if err != nil {
+ return nil, false
+ }
+ location = filepath.join({dir, "review", "answers.json"}, allocator) or_else ""
+ } else {
+ location = strings.clone(path, allocator)
+ }
+ c = new(Cache, allocator)
+ c.path = location
+ c.fresh = fresh
+ c.entries = make(map[string]Entry, allocator)
+ data, err := os.read_entire_file_from_path(location, context.temp_allocator)
+ if err != nil {
+ return c, true // No cache yet, which is not an error.
+ }
+ loaded: map[string]Entry
+ if json.unmarshal(data, &loaded, allocator = allocator) == nil {
+ c.entries = loaded
+ }
+ return c, true
+}
+
+// key is the question, hashed with the provider that was asked and the
+// cache's version.
+key :: proc(provider_name, system, user: string, allocator := context.allocator) -> string {
+ joined := strings.join({version, provider_name, system, user}, "\x00", context.temp_allocator)
+ ctx: sha2.Context_256
+ sha2.init_256(&ctx)
+ sha2.update(&ctx, transmute([]byte)joined)
+ digest: [32]byte
+ sha2.final(&ctx, digest[:])
+ return string(hex.encode(digest[:], allocator))
+}
+
+// get returns the answer recorded for exactly this question, marked as a
+// replay: the run asked nothing, so the replay's usage counts stay at
+// zero.
+get :: proc(
+ c: ^Cache,
+ provider_name, system, user: string,
+ allocator := context.allocator,
+) -> (
+ provider.Answer,
+ bool,
+) {
+ if c == nil {
+ return {}, false
+ }
+ sync.mutex_lock(&c.lock)
+ defer sync.mutex_unlock(&c.lock)
+ if c.fresh {
+ return {}, false
+ }
+ entry, found := c.entries[key(provider_name, system, user, context.temp_allocator)]
+ if !found {
+ return {}, false
+ }
+ c.hits += 1
+ return provider.Answer{text = strings.clone(entry.text, allocator), replayed = true}, true
+}
+
+// put records an answer under its question.
+put :: proc(c: ^Cache, provider_name, system, user: string, answer: provider.Answer) {
+ if c == nil {
+ return
+ }
+ sync.mutex_lock(&c.lock)
+ defer sync.mutex_unlock(&c.lock)
+ c.entries[key(provider_name, system, user)] = Entry {
+ text = strings.clone(answer.text),
+ tokens_in = answer.tokens_in,
+ tokens_out = answer.tokens_out,
+ cached = answer.cached,
+ cost = answer.cost,
+ at = time.time_to_unix(time.now()),
+ }
+ c.dirty = true
+}
+
+// save writes the file back when this run recorded anything, dropping the
+// oldest entries past the bound. A failed write stays silent: the cache
+// is a saving, not a result.
+save :: proc(c: ^Cache) {
+ if c == nil {
+ return
+ }
+ sync.mutex_lock(&c.lock)
+ defer sync.mutex_unlock(&c.lock)
+ if !c.dirty {
+ return
+ }
+ if len(c.entries) > bound {
+ Aged :: struct {
+ key: string,
+ at: i64,
+ }
+ ages := make([dynamic]Aged, context.temp_allocator)
+ for k, e in c.entries {
+ append(&ages, Aged{k, e.at})
+ }
+ slice.sort_by_cmp(ages[:], proc(a, b: Aged) -> slice.Ordering {
+ return .Less if a.at < b.at else (.Greater if a.at > b.at else .Equal)
+ })
+ for a in ages[:len(ages) - bound] {
+ delete_key(&c.entries, a.key)
+ }
+ }
+ data, err := json.marshal(c.entries, allocator = context.temp_allocator)
+ if err != nil {
+ return
+ }
+ if os.make_directory_all(filepath.dir(c.path)) != nil {
+ return
+ }
+ temp := strings.concatenate({c.path, ".tmp"}, context.temp_allocator)
+ if os.write_entire_file(temp, data) != nil {
+ return
+ }
+ os.rename(temp, c.path)
+ c.dirty = false
+}
diff --git a/odin/cache/cache_test.odin b/odin/cache/cache_test.odin
@@ -0,0 +1,43 @@
+package cache
+
+import "core:os"
+import "core:path/filepath"
+import "core:testing"
+
+import "../provider"
+
+@(test)
+answers_replay_across_opens :: proc(t: ^testing.T) {
+ context.allocator = context.temp_allocator
+ temp := os.temp_directory(context.temp_allocator) or_else ""
+ dir, err := os.make_directory_temp(temp, "review-cache-*", context.temp_allocator)
+ testing.expect(t, err == nil)
+ defer os.remove_all(dir)
+ path := filepath.join({dir, "deep", "answers.json"}, context.temp_allocator) or_else ""
+
+ c, ok := open(false, path)
+ testing.expect(t, ok)
+ _, hit := get(c, "p", "s", "u")
+ testing.expect(t, !hit, "an empty cache replays nothing")
+ put(c, "p", "s", "u", provider.Answer{text = "answer", tokens_in = 3, cost = 0.1})
+ got, replayed := get(c, "p", "s", "u")
+ testing.expect(t, replayed)
+ testing.expect_value(t, got.text, "answer")
+ testing.expect(t, got.replayed)
+ testing.expect_value(t, got.tokens_in, 0)
+ save(c)
+
+ again, opened := open(false, path)
+ testing.expect(t, opened)
+ got, replayed = get(again, "p", "s", "u")
+ testing.expect(t, replayed, "the saved answer is read back")
+ testing.expect_value(t, got.text, "answer")
+ _, other := get(again, "other", "s", "u")
+ testing.expect(t, !other, "another provider's question is another question")
+
+ fresh, _ := open(true, path)
+ _, skipped := get(fresh, "p", "s", "u")
+ testing.expect(t, !skipped, "fresh reads nothing")
+ testing.expect(t, key("a", "b", "c") != key("a", "b", "d"))
+ testing.expect_value(t, len(key("a", "b", "c")), 64)
+}
diff --git a/odin/change/change.odin b/odin/change/change.odin
@@ -124,6 +124,12 @@ Change :: struct {
// index is every declaration in the repository at the end of the
// change, kept for the checks that judge new work against it.
index: []Declared,
+ // candidates are existing names that resemble each new one, gathered
+ // by search rather than by the model; twins are the existing
+ // declarations holding the same literal as a new one, the shortest
+ // list worth reading.
+ candidates: map[string][]string,
+ twins: map[string][]string,
}
max_diff :: 60000
diff --git a/odin/change/change_test.odin b/odin/change/change_test.odin
@@ -8,6 +8,7 @@ import "jfm:sh"
import "../frontend"
import "../tree"
+import "../txt"
canned :: `diff --git a/a.go b/a.go
index 1..2 100644
@@ -266,3 +267,70 @@ gather_of_nothing_staged_is_empty :: proc(t: ^testing.T) {
testing.expect_value(t, c.diff, "")
testing.expect_value(t, c.message, "")
}
+
+@(test)
+candidates_rank_by_shared_words :: proc(t: ^testing.T) {
+ context.allocator = context.temp_allocator
+ c := Change{}
+ c.symbols = make([dynamic]Symbol, context.temp_allocator)
+ append(
+ &c.symbols,
+ Symbol {
+ name = "IconEntrySize",
+ kind = "value",
+ file = "b.go",
+ line = 9,
+ signature = "const IconEntrySize = 16",
+ },
+ )
+ c.index = []Declared {
+ {
+ name = "EntrySize",
+ kind = "value",
+ file = "a.go",
+ line = 3,
+ text = "const EntrySize = 16",
+ },
+ {
+ name = "iconCount",
+ kind = "func",
+ file = "a.go",
+ line = 8,
+ text = "func iconCount() int {",
+ },
+ {
+ name = "unrelated",
+ kind = "value",
+ file = "a.go",
+ line = 1,
+ text = "const unrelated = 16",
+ },
+ {
+ name = "IconEntrySize",
+ kind = "value",
+ file = "b.go",
+ line = 9,
+ text = "const IconEntrySize = 16",
+ },
+ }
+ find_candidates(&c, context.temp_allocator)
+ twins := c.twins["IconEntrySize"]
+ testing.expect_value(t, len(twins), 2)
+ if len(twins) == 2 {
+ testing.expect_value(t, twins[0], "a.go:3: const EntrySize = 16 <- same value")
+ }
+ near := c.candidates["IconEntrySize"]
+ testing.expect_value(t, len(near), 4)
+ if len(near) == 4 {
+ testing.expect_value(t, near[2], "a.go:3: const EntrySize = 16")
+ testing.expect_value(t, near[3], "a.go:8: func iconCount() int {")
+ }
+ testing.expect(t, same("x = 3", "y = 3 "))
+ testing.expect(t, same("A :: 3", "B = 3"))
+ testing.expect(t, !same("f()", "g()"))
+ testing.expect_value(
+ t,
+ fmt.tprint(txt.split_words("icoEntry_size")),
+ `["ico", "Entry", "size"]`,
+ )
+}
diff --git a/odin/change/index.odin b/odin/change/index.odin
@@ -0,0 +1,145 @@
+package change
+
+// For every new name, the existing declarations that might already mean
+// the same thing. Finding them is the cheap part; judging the shortlist is
+// what the duplication job is for.
+
+import "core:fmt"
+import "core:slice"
+import "core:strings"
+
+import "../txt"
+
+// shortlist is how many resembling declarations a new name is shown
+// beside, ranked; the twins come before them and are not counted.
+shortlist :: 12
+
+// find_candidates gathers, per new name, the declarations holding the
+// same literal — the strongest hint a fact has been written twice — and
+// then the ones whose names share a word. Both are kept on the change,
+// keyed by the new name.
+find_candidates :: proc(c: ^Change, allocator := context.allocator) {
+ context.allocator = allocator
+ c.twins = make(map[string][]string)
+ c.candidates = make(map[string][]string)
+ for symbol in c.symbols {
+ twins := make([dynamic]string)
+ for declared in c.index {
+ if declared.file == symbol.file && declared.line == symbol.line {
+ continue
+ }
+ if same(declared.text, symbol.signature) {
+ append(
+ &twins,
+ fmt.aprintf("%s <- same value", describe(declared, context.temp_allocator)),
+ )
+ }
+ }
+ if len(twins) > 0 {
+ c.twins[symbol.name] = twins[:]
+ }
+ near := resembling(c.index, symbol, shortlist)
+ listed := make([dynamic]string)
+ append(&listed, ..twins[:])
+ append(&listed, ..near)
+ c.candidates[symbol.name] = listed[:]
+ }
+}
+
+// describe is a declaration as the shortlist names it: where it is, and
+// the line it was declared on.
+describe :: proc(d: Declared, allocator := context.allocator) -> string {
+ return fmt.aprintf(
+ "%s:%d: %s",
+ d.file,
+ d.line,
+ strings.trim_space(d.text),
+ allocator = allocator,
+ )
+}
+
+// resembling returns the declarations whose names share a word with the
+// one given, which is the shortlist a reader is asked to judge. Words are
+// matched whole, so that cache does not pull in every Cached, and the
+// list is ranked: the more words shared the higher, a declaration of the
+// same kind above one of another, and the same file above the rest.
+resembling :: proc(
+ index: []Declared,
+ symbol: Symbol,
+ limit: int,
+ allocator := context.allocator,
+) -> []string {
+ wanted := make(map[string]bool, context.temp_allocator)
+ for word in txt.split_words(symbol.name, context.temp_allocator) {
+ if len(word) >= 4 {
+ wanted[txt.depluralise(strings.to_lower(word, context.temp_allocator))] = true
+ }
+ }
+ if len(wanted) == 0 {
+ return nil
+ }
+ Candidate :: struct {
+ line: string,
+ score: int,
+ order: int,
+ }
+ out := make([dynamic]Candidate, context.temp_allocator)
+ seen := make(map[string]bool, context.temp_allocator)
+ for declared, i in index {
+ if declared.file == symbol.file && declared.line == symbol.line {
+ continue
+ }
+ shared := 0
+ for word in txt.split_words(declared.name, context.temp_allocator) {
+ if wanted[txt.depluralise(strings.to_lower(word, context.temp_allocator))] {
+ shared += 1
+ }
+ }
+ if shared == 0 {
+ continue
+ }
+ line := describe(declared, context.temp_allocator)
+ if seen[line] {
+ continue
+ }
+ seen[line] = true
+ score := shared * 4
+ if declared.kind == symbol.kind {
+ score += 2
+ }
+ if declared.file == symbol.file {
+ score += 1
+ }
+ append(&out, Candidate{line, score, i})
+ }
+ slice.sort_by_cmp(out[:], proc(a, b: Candidate) -> slice.Ordering {
+ if a.score != b.score {
+ return .Less if a.score > b.score else .Greater
+ }
+ return .Less if a.order < b.order else (.Greater if a.order > b.order else .Equal)
+ })
+ kept := out[:min(len(out), limit)]
+ lines := make([]string, len(kept), allocator)
+ for r, i in kept {
+ lines[i] = strings.clone(r.line, allocator)
+ }
+ return lines
+}
+
+// same reports whether two declaration lines hold the same literal. Go
+// and TypeScript write `name = value`; Odin writes `name :: value`.
+same :: proc(a, b: string) -> bool {
+ left, ok_a := literal_of(a)
+ right, ok_b := literal_of(b)
+ return ok_a && ok_b && strings.trim_space(left) == strings.trim_space(right)
+}
+
+literal_of :: proc(text: string) -> (string, bool) {
+ if i := strings.index_byte(text, '='); i >= 0 {
+ return text[i + 1:], true
+ }
+ if i := strings.index(text, "::"); i >= 0 {
+ return text[i + 2:], true
+ }
+ return "", false
+}
diff --git a/odin/check/check.odin b/odin/check/check.odin
@@ -169,27 +169,6 @@ set :: proc(words: string) -> map[string]bool {
return out
}
-// split breaks a name into its words: a capital starts a word, an
-// underscore ends one, as snake_case languages put the next word after
-// it.
-split :: proc(name: string, allocator := context.allocator) -> []string {
- words := make([dynamic]string, allocator)
- start := 0
- for i in 0 ..< len(name) {
- c := name[i]
- if i > 0 && ((c >= 'A' && c <= 'Z') || c == '_') {
- if i > start {
- append(&words, name[start:i])
- }
- start = i + 1 if c == '_' else i
- }
- }
- if start < len(name) {
- append(&words, name[start:])
- }
- return words[:]
-}
-
// fields splits on anything that is not a letter or a digit.
fields :: proc(s: string, allocator := context.allocator) -> []string {
out := make([dynamic]string, allocator)
diff --git a/odin/check/check_test.odin b/odin/check/check_test.odin
@@ -12,6 +12,7 @@ import "../change"
import "../finding"
import "../frontend"
import "../tree"
+import "../txt"
// dyn is a dynamic array over the items given, for a change built in a
@@ -554,7 +555,7 @@ names_are_measured :: proc(t: ^testing.T) {
)
testing.expect_value(
t,
- fmt.tprint(split("parseHTTPRequest_now", context.temp_allocator)),
+ fmt.tprint(txt.split_words("parseHTTPRequest_now", context.temp_allocator)),
`["parse", "H", "T", "T", "P", "Request", "now"]`,
)
}
diff --git a/odin/check/gaming.odin b/odin/check/gaming.odin
@@ -9,6 +9,7 @@ import "core:strings"
import "../change"
import "../finding"
+import "../txt"
// rule_id is what a dismissal's rule id has to look like. The prose that
// documents the mechanism writes placeholders (<rule>) and quoted
@@ -225,7 +226,7 @@ covered :: proc(deleted: string, spoken: []string) -> bool {
// short leftovers left out.
test_words :: proc(name: string, allocator := context.allocator) -> []string {
out := make([dynamic]string, allocator)
- for part in split(name, context.temp_allocator) {
+ for part in txt.split_words(name, context.temp_allocator) {
for piece in strings.fields(part, context.temp_allocator) {
word := strings.to_lower(piece, allocator)
if len(word) < 3 {
diff --git a/odin/check/message.odin b/odin/check/message.odin
@@ -13,6 +13,7 @@ import "vendor:zlib"
import "../change"
import "../finding"
+import "../txt"
// min_entropy is the Shannon entropy, in bits per byte, under which a
// message is one short phrase said over rather than a description of a
@@ -625,7 +626,7 @@ content_words :: proc(text: string, allocator := context.allocator) -> map[strin
out := make(map[string]bool, allocator)
for piece in fields(text, context.temp_allocator) {
for part in humps(piece, context.temp_allocator) {
- w := depluralise(strings.to_lower(part, allocator))
+ w := txt.depluralise(strings.to_lower(part, allocator))
if len(w) > 2 && !stop_words[w] {
out[w] = true
}
@@ -654,15 +655,6 @@ humps :: proc(s: string, allocator := context.allocator) -> []string {
return out[:]
}
-// depluralise drops a trailing s, except where dropping it would leave
-// another: class stays class.
-depluralise :: proc(w: string) -> string {
- if len(w) > 3 && strings.has_suffix(w, "s") && w[len(w) - 2] != 's' {
- return w[:len(w) - 1]
- }
- return w
-}
-
// frequencies counts, over the repository's subjects, in how many of them
// each content word appears.
frequencies :: proc(history: []string, allocator := context.allocator) -> map[string]int {
diff --git a/odin/check/names.odin b/odin/check/names.odin
@@ -10,6 +10,7 @@ import "core:strings"
import "../change"
import "../finding"
+import "../txt"
@(private = "file")
predeclared: map[string]bool
@@ -81,7 +82,7 @@ stutter :: proc(sym: change.Symbol, out: ^[dynamic]finding.Finding) {
if sym.pkg == "" || sym.pkg == "main" || !sym.exported {
return
}
- words := split(sym.name, context.temp_allocator)
+ words := txt.split_words(sym.name, context.temp_allocator)
if len(words) < 2 {
return
}
@@ -161,7 +162,7 @@ shadow :: proc(sym: change.Symbol, out: ^[dynamic]finding.Finding) {
// the reader has to expand rather than read.
abbreviated :: proc(sym: change.Symbol, out: ^[dynamic]finding.Finding) {
hit := make([dynamic]string, context.temp_allocator)
- for word in split(sym.name, context.temp_allocator) {
+ for word in txt.split_words(sym.name, context.temp_allocator) {
if abbreviations[strings.to_lower(word, context.temp_allocator)] {
append(&hit, word)
}
diff --git a/odin/job/job.odin b/odin/job/job.odin
@@ -0,0 +1,530 @@
+/*
+Package job is the narrow readings a model is asked for. Each is given the
+part of the change it needs and nothing else: a job that reads less is
+cheaper, and harder to distract into reporting something another job
+owns. The criteria a job judges against are the thing to tune when it
+reports the wrong things, and they travel with the binary.
+*/
+package job
+
+import "base:runtime"
+import "core:encoding/json"
+import "core:fmt"
+import "core:slice"
+import "core:strings"
+import "core:text/regex"
+
+import "../change"
+import "../check"
+import "../finding"
+import "../txt"
+
+// Job is one reading. subject renders the part of the change it reads;
+// an empty subject means there is nothing here for it and the job is
+// skipped. splittable is whether the subject can be read file by file: a
+// subject over the packet cap is then asked in parts.
+Job :: struct {
+ name: string,
+ criteria: string,
+ subject: proc(c: ^change.Change, allocator: runtime.Allocator) -> string,
+ splittable: bool,
+}
+
+// all is the readings, in the order their findings are worth having.
+all :: proc(allocator := context.temp_allocator) -> []Job {
+ jobs := make([]Job, 5, allocator)
+ jobs[0] = Job {
+ "duplication",
+ #load("../../criteria/duplication.md", string),
+ duplication_subject,
+ true,
+ }
+ jobs[1] = Job{"tests", #load("../../criteria/tests.md", string), tests_subject, true}
+ jobs[2] = Job{"namer", #load("../../criteria/namer.md", string), namer_subject, true}
+ jobs[3] = Job{"claims", #load("../../criteria/claims.md", string), claims_subject, true}
+ jobs[4] = Job{"hygiene", #load("../../criteria/hygiene.md", string), hygiene_subject, false}
+ return jobs
+}
+
+// chosen is the jobs named, comma separated, or all of them.
+chosen :: proc(only: string, allocator := context.temp_allocator) -> (jobs: []Job, err: string) {
+ if strings.trim_space(only) == "" {
+ return all(allocator), ""
+ }
+ picked := make([dynamic]Job, allocator)
+ for name in strings.split(only, ",", context.temp_allocator) {
+ want := strings.trim_space(name)
+ found := false
+ for j in all(allocator) {
+ if j.name == want {
+ append(&picked, j)
+ found = true
+ }
+ }
+ if !found {
+ return nil, fmt.aprintf(
+ "no job called %q; the jobs are claims, duplication, hygiene, namer, tests",
+ want,
+ allocator = allocator,
+ )
+ }
+ }
+ return picked[:], ""
+}
+
+// rules reads the ids a job may cite out of its own criteria: the bullets
+// opening with a backticked id.
+rules :: proc(criteria: string, allocator := context.temp_allocator) -> map[string]bool {
+ out := make(map[string]bool, allocator)
+ rest := criteria
+ for line in strings.split_lines_iterator(&rest) {
+ if !strings.has_prefix(line, "- `") {
+ continue
+ }
+ end := strings.index_byte(line[3:], '`')
+ if end < 0 {
+ continue
+ }
+ id := line[3:3 + end]
+ valid := len(id) > 0
+ for i in 0 ..< len(id) {
+ c := id[i]
+ if !((c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '-') {
+ valid = false
+ }
+ }
+ if valid {
+ out[id] = true
+ }
+ }
+ return out
+}
+
+namer_subject :: proc(c: ^change.Change, allocator: runtime.Allocator) -> string {
+ if len(c.symbols) == 0 {
+ return ""
+ }
+ b := strings.builder_make(allocator)
+ strings.write_string(&b, "Names this change adds or renames:\n\n")
+ for s in c.symbols {
+ fmt.sbprintf(&b, "%s:%d %s %s", s.file, s.line, s.kind, s.name)
+ if s.exported {
+ strings.write_string(&b, " (exported)")
+ }
+ strings.write_string(&b, "\n")
+ if s.signature != "" {
+ fmt.sbprintf(&b, " %s\n", s.signature)
+ }
+ if s.doc != "" {
+ fmt.sbprintf(&b, " doc: %s\n", check.first_line(s.doc, context.temp_allocator))
+ }
+ if near := c.candidates[s.name]; len(near) > 0 {
+ fmt.sbprintf(
+ &b,
+ " names already in this repository: %s\n",
+ strings.join(near[:min(len(near), 6)], "; ", context.temp_allocator),
+ )
+ }
+ strings.write_string(&b, "\n")
+ }
+ return strings.to_string(b)
+}
+
+duplication_subject :: proc(c: ^change.Change, allocator: runtime.Allocator) -> string {
+ if len(c.symbols) == 0 {
+ return ""
+ }
+ b := strings.builder_make(allocator)
+ // The pairs holding the same literal go first and alone. Buried among
+ // the resemblances a reading finds one of them and stops.
+ if twinned := twins(c, context.temp_allocator); twinned != "" {
+ strings.write_string(
+ &b,
+ "Declarations this change adds that hold a value already declared elsewhere.\n",
+ )
+ strings.write_string(&b, "Judge every pair on this list.\n\n")
+ strings.write_string(&b, twinned)
+ strings.write_string(&b, "\n")
+ }
+ strings.write_string(
+ &b,
+ "Each name the change adds, with existing declarations found by searching for its words.\n\n",
+ )
+ for s in c.symbols {
+ fmt.sbprintf(&b, "NEW %s:%d %s %s\n", s.file, s.line, s.kind, s.name)
+ if s.signature != "" {
+ fmt.sbprintf(&b, " %s\n", s.signature)
+ }
+ if s.doc != "" {
+ fmt.sbprintf(&b, " doc: %s\n", check.first_line(s.doc, context.temp_allocator))
+ }
+ candidates := c.candidates[s.name]
+ if len(candidates) == 0 {
+ strings.write_string(&b, " candidates: none found\n\n")
+ continue
+ }
+ strings.write_string(&b, " candidates:\n")
+ for line in candidates {
+ fmt.sbprintf(&b, " %s\n", line)
+ }
+ strings.write_string(&b, "\n")
+ }
+ return strings.to_string(b)
+}
+
+// twins renders the declarations whose value already exists, in the
+// order the change declares them.
+twins :: proc(c: ^change.Change, allocator := context.allocator) -> string {
+ b := strings.builder_make(allocator)
+ for s in c.symbols {
+ lines := c.twins[s.name]
+ if len(lines) == 0 {
+ continue
+ }
+ fmt.sbprintf(&b, " %s:%d %s\n", s.file, s.line, s.signature)
+ for line in lines {
+ fmt.sbprintf(&b, " %s\n", strings.trim_suffix(line, " <- same value"))
+ }
+ }
+ return strings.to_string(b)
+}
+
+tests_subject :: proc(c: ^change.Change, allocator: runtime.Allocator) -> string {
+ if len(c.tests) == 0 {
+ return ""
+ }
+ b := strings.builder_make(allocator)
+ strings.write_string(&b, "Test functions this change adds or alters:\n\n")
+ for t in c.tests {
+ fmt.sbprintf(&b, "--- %s:%d %s", t.file, t.line, t.name)
+ if t.skips > 0 {
+ // The skip is pointed at rather than left to be found, so the
+ // reading spends itself on whether the skip is ordinary.
+ fmt.sbprintf(&b, " (skips itself at line %d)", t.skips)
+ }
+ fmt.sbprintf(&b, "\n%s\n\n", t.body)
+ }
+ if called := functions_under_test(c, context.temp_allocator); called != "" {
+ strings.write_string(
+ &b,
+ "Functions the tests call, as they stand at the end of the change. A test that\n",
+ )
+ strings.write_string(
+ &b,
+ "would pass with one of these returning its input or a zero value is the finding.\n\n",
+ )
+ strings.write_string(&b, called)
+ }
+ return strings.to_string(b)
+}
+
+// The bounds on what the tests job is shown of the code under test: how
+// many functions, and how long each may be before it is cut.
+called_functions :: 8
+called_lines :: 60
+
+// functions_under_test renders the functions the tests call, found by
+// name in the repository's index, so that whether a test would pass on a
+// stub is judged against the function rather than guessed from the test.
+// A helper a test file declares is not the code under test.
+functions_under_test :: proc(c: ^change.Change, allocator := context.allocator) -> string {
+ if len(c.index) == 0 {
+ return ""
+ }
+ declared := make(map[string]change.Declared, context.temp_allocator)
+ for d in c.index {
+ if d.kind == "func" && d.body != "" && !check.is_test_file(d.file) {
+ if d.name not_in declared {
+ declared[d.name] = d
+ }
+ }
+ }
+ seen := make(map[string]bool, context.temp_allocator)
+ shown := make([dynamic]change.Declared, context.temp_allocator)
+ outer: for t in c.tests {
+ for name in calls(t.body, context.temp_allocator) {
+ d, ok := declared[name]
+ if !ok || seen[name] || name == t.name {
+ continue
+ }
+ seen[name] = true
+ append(&shown, d)
+ if len(shown) == called_functions {
+ break outer
+ }
+ }
+ }
+ if len(shown) == 0 {
+ return ""
+ }
+ b := strings.builder_make(allocator)
+ for d in shown {
+ body := d.body
+ lines := strings.split_lines(body, context.temp_allocator)
+ if len(lines) > called_lines {
+ body = fmt.tprintf(
+ "%s\n\t… cut at %d lines",
+ strings.join(lines[:called_lines], "\n", context.temp_allocator),
+ called_lines,
+ )
+ }
+ fmt.sbprintf(&b, "--- %s:%d %s\n%s\n\n", d.file, d.line, d.name, body)
+ }
+ return strings.to_string(b)
+}
+
+// calls are the names a body calls, in order, which is how a test names
+// what it tests.
+calls :: proc(body: string, allocator := context.allocator) -> []string {
+ out := make([dynamic]string, allocator)
+ it, err := regex.create_iterator(
+ body,
+ `\b([A-Za-z_][A-Za-z0-9_]*)\(`,
+ {},
+ context.temp_allocator,
+ )
+ if err != nil {
+ return out[:]
+ }
+ defer regex.destroy_iterator(it, context.temp_allocator)
+ for {
+ cap, _, ok := regex.match_iterator(&it)
+ if !ok {
+ break
+ }
+ append(&out, strings.trim_suffix(cap.groups[0], "("))
+ }
+ return out[:]
+}
+
+claims_subject :: proc(c: ^change.Change, allocator: runtime.Allocator) -> string {
+ blocks := comment_blocks(c.comments[:], context.temp_allocator)
+ if len(blocks) == 0 {
+ return ""
+ }
+ b := strings.builder_make(allocator)
+ strings.write_string(
+ &b,
+ "Comment and documentation lines this change adds, each with the code beneath it:\n\n",
+ )
+ for block in blocks {
+ for comment in block {
+ fmt.sbprintf(&b, "%s:%d %s\n", comment.file, comment.line, comment.text)
+ }
+ if below := block[len(block) - 1].below; below != "" {
+ rest := below
+ for line in strings.split_lines_iterator(&rest) {
+ fmt.sbprintf(&b, " code: %s\n", line)
+ }
+ }
+ strings.write_string(&b, "\n")
+ }
+ return strings.to_string(b)
+}
+
+// comment_blocks groups the comments into the runs of consecutive lines
+// they were written as, so a claim read over three lines is read whole
+// and the code below it is shown once. A comment whose words are the
+// code's own is left out: it is never a claim, and measured elsewhere.
+comment_blocks :: proc(
+ comments: []change.Located,
+ allocator := context.allocator,
+) -> [][]change.Located {
+ blocks := make([dynamic][]change.Located, allocator)
+ current := make([dynamic]change.Located, allocator)
+ for comment in comments {
+ if check.restates(comment) {
+ continue
+ }
+ if len(current) > 0 {
+ last := current[len(current) - 1]
+ if !(last.file == comment.file && last.line + 1 == comment.line) {
+ append(&blocks, current[:])
+ current = make([dynamic]change.Located, allocator)
+ }
+ }
+ append(¤t, comment)
+ }
+ if len(current) > 0 {
+ append(&blocks, current[:])
+ }
+ return blocks[:]
+}
+
+hygiene_subject :: proc(c: ^change.Change, allocator: runtime.Allocator) -> string {
+ if strings.trim_space(c.message) == "" {
+ return ""
+ }
+ b := strings.builder_make(allocator)
+ strings.write_string(&b, "Commit message:\n\n")
+ strings.write_string(&b, c.message)
+ strings.write_string(&b, "\n\nFiles changed:\n")
+ strings.write_string(&b, c.stat)
+ if len(c.convention) > 0 {
+ strings.write_string(
+ &b,
+ "\nRecent subjects in this repository, as the local convention:\n",
+ )
+ for subject in c.convention {
+ fmt.sbprintf(&b, " %s\n", subject)
+ }
+ }
+ return strings.to_string(b)
+}
+
+// Reported is the shape a job answers in.
+Reported :: struct {
+ findings: []struct {
+ rule: string `json:"rule"`,
+ severity: string `json:"severity"`,
+ file: string `json:"file"`,
+ line: int `json:"line"`,
+ symbol: string `json:"symbol"`,
+ message: string `json:"message"`,
+ fix: string `json:"fix"`,
+ } `json:"findings"`,
+}
+
+// decode reads a job's answer. A finding that cites no rule from the
+// criteria is dropped: the criteria are what gets tuned, so a job may not
+// invent one.
+decode :: proc(
+ raw: string,
+ name: string,
+ allowed: map[string]bool,
+ allocator := context.allocator,
+) -> (
+ out: []finding.Finding,
+ ok: bool,
+) {
+ r: Reported
+ if json.unmarshal_string(raw, &r, allocator = context.temp_allocator) != nil {
+ return nil, false
+ }
+ kept := make([dynamic]finding.Finding, allocator)
+ for f in r.findings {
+ if !allowed[f.rule] {
+ continue
+ }
+ append(
+ &kept,
+ finding.Finding {
+ job = strings.clone(name, allocator),
+ rule = strings.clone(f.rule, allocator),
+ severity = finding.parse_severity(f.severity),
+ severity_name = strings.clone(f.severity, allocator),
+ file = strings.clone(f.file, allocator),
+ line = f.line,
+ symbol = strings.clone(f.symbol, allocator),
+ message = strings.clone(f.message, allocator),
+ fix = strings.clone(f.fix, allocator),
+ },
+ )
+ }
+ return kept[:], true
+}
+
+// readable is whether an answer holds a findings object at all.
+readable :: proc(text: string) -> bool {
+ raw, found := txt.object(text)
+ if !found {
+ return false
+ }
+ r: Reported
+ return json.unmarshal_string(raw, &r, allocator = context.temp_allocator) == nil
+}
+
+// packet_cap is the size of subject past which a splittable job is asked
+// in parts. Under the cap each part is an ask a slow gateway finishes,
+// and a part whose files did not change replays from the cache.
+packet_cap :: 16000
+
+// parts is the subjects a job is asked, as changes: the whole change when
+// it fits or cannot be split, else the change cut file by file into runs
+// that each render under the cap. A file that alone renders over the cap
+// is a part by itself.
+parts :: proc(j: Job, c: ^change.Change, allocator := context.allocator) -> []^change.Change {
+ out := make([dynamic]^change.Change, allocator)
+ if !j.splittable || len(j.subject(c, context.temp_allocator)) <= packet_cap {
+ append(&out, c)
+ return out[:]
+ }
+ group := make([dynamic]string, context.temp_allocator)
+ for file in c.files {
+ if !contributes(c, file) {
+ continue
+ }
+ if len(group) > 0 {
+ trial := slice.clone(group[:], context.temp_allocator)
+ with := make([dynamic]string, context.temp_allocator)
+ append(&with, ..trial)
+ append(&with, file)
+ piece := part(c, with[:], context.temp_allocator)
+ if len(j.subject(piece, context.temp_allocator)) > packet_cap {
+ append(&out, part(c, group[:], allocator))
+ clear(&group)
+ }
+ }
+ append(&group, file)
+ }
+ if len(group) > 0 {
+ append(&out, part(c, group[:], allocator))
+ }
+ return out[:]
+}
+
+// contributes is whether a file has anything a splittable job reads.
+contributes :: proc(c: ^change.Change, file: string) -> bool {
+ for s in c.symbols {
+ if s.file == file {
+ return true
+ }
+ }
+ for t in c.tests {
+ if t.file == file {
+ return true
+ }
+ }
+ for comment in c.comments {
+ if comment.file == file {
+ return true
+ }
+ }
+ return false
+}
+
+// part is the change narrowed to some of its files: the declarations,
+// tests and comments in them, with everything the jobs read beside those
+// — candidates, twins, the index, the message — shared.
+part :: proc(
+ c: ^change.Change,
+ files: []string,
+ allocator := context.allocator,
+) -> ^change.Change {
+ keep := make(map[string]bool, context.temp_allocator)
+ for f in files {
+ keep[f] = true
+ }
+ p := new(change.Change, allocator)
+ p^ = c^
+ p.files = slice.clone(files, allocator)
+ p.symbols = make([dynamic]change.Symbol, allocator)
+ p.tests = make([dynamic]change.Function, allocator)
+ p.comments = make([dynamic]change.Located, allocator)
+ for s in c.symbols {
+ if keep[s.file] {
+ append(&p.symbols, s)
+ }
+ }
+ for t in c.tests {
+ if keep[t.file] {
+ append(&p.tests, t)
+ }
+ }
+ for comment in c.comments {
+ if keep[comment.file] {
+ append(&p.comments, comment)
+ }
+ }
+ return p
+}
diff --git a/odin/job/job_test.odin b/odin/job/job_test.odin
@@ -0,0 +1,199 @@
+package job
+
+import "core:strings"
+import "core:testing"
+
+import "../change"
+import "../txt"
+
+dyn :: proc(items: []$T) -> [dynamic]T {
+ out := make([dynamic]T, context.temp_allocator)
+ append(&out, ..items)
+ return out
+}
+
+@(test)
+jobs_carry_their_criteria :: proc(t: ^testing.T) {
+ jobs := all(context.temp_allocator)
+ testing.expect_value(t, len(jobs), 5)
+ for j in jobs {
+ testing.expectf(t, len(rules(j.criteria)) >= 4, "%s cites too few rules", j.name)
+ testing.expect(t, strings.contains(j.criteria, "- `"))
+ }
+ allowed := rules(jobs[1].criteria)
+ testing.expect(t, allowed["cannot-fail"], "the tests criteria name cannot-fail")
+ picked, err := chosen("tests, hygiene", context.temp_allocator)
+ testing.expect_value(t, err, "")
+ testing.expect_value(t, len(picked), 2)
+ _, err = chosen("nothing", context.temp_allocator)
+ testing.expect(t, strings.contains(err, "no job called"))
+}
+
+@(test)
+subjects_render_the_part_each_job_reads :: proc(t: ^testing.T) {
+ context.allocator = context.temp_allocator
+ c := change.Change {
+ message = "x: do the thing",
+ stat = " a.go | 2 +-",
+ convention = {"x: earlier"},
+ symbols = dyn(
+ []change.Symbol {
+ {
+ name = "Parse",
+ kind = "func",
+ file = "a.go",
+ line = 3,
+ exported = true,
+ signature = "func Parse() {",
+ doc = "Parse reads.\nMore.",
+ },
+ },
+ ),
+ tests = dyn(
+ []change.Function {
+ {
+ name = "TestParse",
+ file = "a_test.go",
+ line = 9,
+ body = "func TestParse(t *testing.T) {\n\tt.Skip()\n\tParse()\n}",
+ skips = 10,
+ },
+ },
+ ),
+ comments = dyn(
+ []change.Located {
+ {text = "one", file = "a.go", line = 1, below = "x := 1"},
+ {text = "two", file = "a.go", line = 2, below = "y := 2\nz := 3"},
+ {text = "parse the input", file = "a.go", line = 7, below = "func parseInput() {"},
+ },
+ ),
+ index = {
+ {
+ name = "Parse",
+ kind = "func",
+ file = "a.go",
+ line = 3,
+ body = "func Parse() {\n\treturn\n}",
+ },
+ },
+ }
+ c.candidates = make(map[string][]string)
+ c.candidates["Parse"] = []string{"b.go:4: func parse() {"}
+ c.twins = make(map[string][]string)
+ c.twins["Parse"] = []string{"b.go:1: const Parse = 1 <- same value"}
+ namer := namer_subject(&c, context.temp_allocator)
+ for want in ([]string{"a.go:3 func Parse (exported)", "doc: Parse reads.", "names already in this repository: b.go:4: func parse() {"}) {
+ testing.expectf(
+ t,
+ strings.contains(namer, want),
+ "%q missing from namer:\n%s",
+ want,
+ namer,
+ )
+ }
+ dup := duplication_subject(&c, context.temp_allocator)
+ for want in ([]string{"Judge every pair on this list.", " b.go:1: const Parse = 1\n", "NEW a.go:3 func Parse", " b.go:4: func parse() {"}) {
+ testing.expectf(
+ t,
+ strings.contains(dup, want),
+ "%q missing from duplication:\n%s",
+ want,
+ dup,
+ )
+ }
+ tests := tests_subject(&c, context.temp_allocator)
+ for want in ([]string{"--- a_test.go:9 TestParse (skips itself at line 10)", "Functions the tests call", "--- a.go:3 Parse\nfunc Parse() {"}) {
+ testing.expectf(
+ t,
+ strings.contains(tests, want),
+ "%q missing from tests:\n%s",
+ want,
+ tests,
+ )
+ }
+ claims := claims_subject(&c, context.temp_allocator)
+ for want in ([]string{"a.go:1 one\na.go:2 two\n code: y := 2\n code: z := 3\n"}) {
+ testing.expectf(
+ t,
+ strings.contains(claims, want),
+ "%q missing from claims:\n%s",
+ want,
+ claims,
+ )
+ }
+ testing.expect(
+ t,
+ !strings.contains(claims, "parse the input"),
+ "a restating comment is not a claim",
+ )
+ hygiene := hygiene_subject(&c, context.temp_allocator)
+ for want in ([]string{"Commit message:\n\nx: do the thing", "Files changed:\n a.go | 2 +-", " x: earlier"}) {
+ testing.expectf(
+ t,
+ strings.contains(hygiene, want),
+ "%q missing from hygiene:\n%s",
+ want,
+ hygiene,
+ )
+ }
+ empty := change.Change{}
+ testing.expect_value(t, namer_subject(&empty, context.temp_allocator), "")
+ testing.expect_value(t, hygiene_subject(&empty, context.temp_allocator), "")
+}
+
+@(test)
+answers_are_decoded_and_held_to_the_criteria :: proc(t: ^testing.T) {
+ allowed := rules("- `cannot-fail` x\n- `other` y\n", context.temp_allocator)
+ raw, found := txt.object(
+ "Sure! {\"findings\":[{\"rule\":\"cannot-fail\",\"severity\":\"must-fix\",\"file\":\"a_test.go\",\"line\":4,\"symbol\":\"TestX\",\"message\":\"m\",\"fix\":\"f\"},{\"rule\":\"invented\",\"severity\":\"note\",\"message\":\"n\"}]} thanks",
+ )
+ testing.expect(t, found)
+ got, ok := decode(raw, "tests", allowed, context.temp_allocator)
+ testing.expect(t, ok)
+ testing.expect_value(t, len(got), 1)
+ if len(got) == 1 {
+ testing.expect_value(t, got[0].job, "tests")
+ testing.expect_value(t, got[0].rule, "cannot-fail")
+ testing.expect_value(t, got[0].line, 4)
+ testing.expect_value(t, got[0].severity_name, "must-fix")
+ }
+ testing.expect(t, readable(`{"findings":[]}`))
+ testing.expect(t, !readable("no json here"))
+ _, none := txt.object("prose")
+ testing.expect(t, !none)
+ testing.expect_value(t, len(calls("f(); g.h(x); if (y) {", context.temp_allocator)), 2)
+}
+
+@(test)
+parts_cut_a_large_subject_by_file :: proc(t: ^testing.T) {
+ context.allocator = context.temp_allocator
+ c := change.Change {
+ files = {"a.go", "b.go", "c.md", "d.go"},
+ }
+ c.symbols = make([dynamic]change.Symbol)
+ long := strings.repeat("x", 9000)
+ for name in ([]string{"a.go", "b.go", "d.go"}) {
+ append(
+ &c.symbols,
+ change.Symbol{name = "S", kind = "func", file = name, line = 1, signature = long},
+ )
+ }
+ j := all(context.temp_allocator)[2]
+ pieces := parts(j, &c, context.temp_allocator)
+ testing.expect_value(t, len(pieces), 3)
+ if len(pieces) == 3 {
+ testing.expect_value(t, len(pieces[0].symbols), 1)
+ testing.expect_value(t, pieces[0].files[0], "a.go")
+ testing.expect_value(t, pieces[2].files[0], "d.go")
+ }
+ small := change.Change {
+ files = {"a.go"},
+ }
+ small.symbols = make([dynamic]change.Symbol)
+ append(
+ &small.symbols,
+ change.Symbol{name = "S", kind = "func", file = "a.go", line = 1, signature = "short"},
+ )
+ testing.expect_value(t, len(parts(j, &small, context.temp_allocator)), 1)
+ testing.expect(t, !contributes(&c, "c.md"))
+}
diff --git a/odin/provider/api.odin b/odin/provider/api.odin
@@ -0,0 +1,199 @@
+package provider
+
+// The console API is the one provider that can enforce the answer's shape
+// rather than request it, so a malformed answer is impossible here rather
+// than merely unlikely: the job answers through a strict tool, and the
+// tool's input is the answer.
+
+import "core:encoding/json"
+import "core:fmt"
+import "core:os"
+import "core:strings"
+import "core:time"
+import "jfm:http"
+
+api_url :: "https://api.anthropic.com/v1/messages"
+api_version :: "2023-06-01"
+
+// Tool is the strict shape one ask is held to.
+Tool :: struct {
+ name, description, schema: string,
+}
+
+// findings_tool is the shape a finding takes.
+findings_tool := Tool {
+ "report_findings",
+ "Report what this reading found, or an empty list.",
+ `{"type":"object","properties":{"findings":{"type":"array","items":{"type":"object","properties":{"rule":{"type":"string","description":"the rule id from the criteria"},"severity":{"type":"string","enum":["must-fix","consider","note"]},"file":{"type":"string"},"line":{"type":"integer"},"symbol":{"type":"string"},"message":{"type":"string","description":"one sentence stating the finding"},"fix":{"type":"string","description":"the concrete change to make"}},"required":["rule","severity","message","fix","file","line","symbol"],"additionalProperties":false}}},"required":["findings"],"additionalProperties":false}`,
+}
+
+// verdicts_tool is the shape the second reading answers in.
+verdicts_tool := Tool {
+ "report_verdicts",
+ "Report whether each finding holds.",
+ `{"type":"object","properties":{"verdicts":{"type":"array","items":{"type":"object","properties":{"index":{"type":"integer","description":"the number of the finding, as it was listed"},"holds":{"type":"boolean","description":"whether the finding stands against the criteria"},"reason":{"type":"string","description":"why it holds or falls, one sentence"}},"required":["index","holds","reason"],"additionalProperties":false}}},"required":["verdicts"],"additionalProperties":false}`,
+}
+
+// request is the body of one ask: the criteria as the system prompt,
+// where the API caches them; the temperature pinned, because a loop
+// between two models cannot converge if one of them answers differently
+// each time it is asked.
+request :: proc(
+ model, system, user: string,
+ tool: Tool,
+ allocator := context.allocator,
+) -> string {
+ return strings.concatenate(
+ {
+ `{"model":`,
+ quote(model),
+ `,"max_tokens":4096,"temperature":0,"system":[{"type":"text","text":`,
+ quote(system),
+ `,"cache_control":{"type":"ephemeral"}}],"tools":[{"name":`,
+ quote(tool.name),
+ `,"description":`,
+ quote(tool.description),
+ `,"strict":true,"input_schema":`,
+ tool.schema,
+ `}],"messages":[{"role":"user","content":[{"type":"text","text":`,
+ quote(user),
+ `}]}]}`,
+ },
+ allocator,
+ )
+}
+
+// quote is a string as JSON writes it.
+quote :: proc(s: string) -> string {
+ data, err := json.marshal(s, allocator = context.temp_allocator)
+ if err != nil {
+ return `""`
+ }
+ return string(data)
+}
+
+// ask_api puts one question with one strict tool. The key is the
+// environment's, as the SDKs read it.
+ask_api :: proc(
+ p: Provider,
+ system, user: string,
+ tool: Tool,
+ allocator := context.allocator,
+) -> (
+ Answer,
+ string,
+) {
+ key := os.get_env("ANTHROPIC_API_KEY", context.temp_allocator)
+ if key == "" {
+ return {}, "no Anthropic credentials found: ANTHROPIC_API_KEY is not set"
+ }
+ body := request(p.model, system, user, tool, context.temp_allocator)
+ res, err := http.post(
+ api_url,
+ body,
+ "application/json",
+ {
+ headers = {
+ strings.concatenate({"x-api-key: ", key}, context.temp_allocator),
+ strings.concatenate({"anthropic-version: ", api_version}, context.temp_allocator),
+ },
+ timeout = time.Duration(ask_timeout_seconds()) * time.Second,
+ },
+ context.temp_allocator,
+ )
+ if err != .None {
+ return {}, fmt.aprintf("api: %v", err, allocator = allocator)
+ }
+ if !res.ok {
+ return {}, fmt.aprintf("api: HTTP %d: %s", res.status, first(error_of(res.body), 200), allocator = allocator)
+ }
+ return reply(res.body, allocator)
+}
+
+// error_of is the message an API error carries, or the body whole.
+error_of :: proc(body: string) -> string {
+ Failure :: struct {
+ error: struct {
+ message: string `json:"message"`,
+ } `json:"error"`,
+ }
+ f: Failure
+ if json.unmarshal_string(body, &f, allocator = context.temp_allocator) == nil &&
+ f.error.message != "" {
+ return f.error.message
+ }
+ return body
+}
+
+// reply reads the tool's input out of the API's answer, which is the
+// answer, and the usage beside it.
+reply :: proc(body: string, allocator := context.allocator) -> (Answer, string) {
+ Response :: struct {
+ content: []struct {
+ type: string `json:"type"`,
+ } `json:"content"`,
+ usage: struct {
+ tokens_in: int `json:"input_tokens"`,
+ tokens_out: int `json:"output_tokens"`,
+ cached: int `json:"cache_read_input_tokens"`,
+ } `json:"usage"`,
+ }
+ r: Response
+ if json.unmarshal_string(body, &r, allocator = context.temp_allocator) != nil {
+ return {}, "api: the answer is not the JSON expected"
+ }
+ answer := Answer {
+ tokens_in = r.usage.tokens_in,
+ tokens_out = r.usage.tokens_out,
+ cached = r.usage.cached,
+ }
+ for block in r.content {
+ if block.type == "tool_use" {
+ if input, found := tool_input(body); found {
+ answer.text = strings.clone(input, allocator)
+ return answer, ""
+ }
+ }
+ }
+ return answer, "the model answered without reporting"
+}
+
+// tool_input is the text of the first "input" object in the answer, as
+// the API wrote it, which is the report the tool was held to.
+tool_input :: proc(body: string) -> (string, bool) {
+ marker :: `"input":`
+ at := strings.index(body, marker)
+ if at < 0 {
+ return "", false
+ }
+ start := at + len(marker)
+ for start < len(body) && (body[start] == ' ' || body[start] == '\n' || body[start] == '\t') {
+ start += 1
+ }
+ if start >= len(body) || body[start] != '{' {
+ return "", false
+ }
+ depth := 0
+ in_string := false
+ for i := start; i < len(body); i += 1 {
+ c := body[i]
+ switch {
+ case in_string:
+ if c == '\\' {
+ i += 1
+ } else if c == '"' {
+ in_string = false
+ }
+ case c == '"':
+ in_string = true
+ case c == '{':
+ depth += 1
+ case c == '}':
+ depth -= 1
+ if depth == 0 {
+ return body[start:i + 1], true
+ }
+ }
+ }
+ return "", false
+}
diff --git a/odin/provider/chain.odin b/odin/provider/chain.odin
@@ -0,0 +1,71 @@
+package provider
+
+// A chain is a priority order of providers. The first one that answers a
+// probe serves the whole reading, so a provider over its limit costs one
+// cheap ask rather than a failed review. Which provider is reachable is a
+// property of the hour: a team limit exhausted, a gateway asleep, a key
+// unset — the chain is how the tool rides that out without being told.
+
+import "core:fmt"
+import "core:os"
+import "core:strings"
+
+// Entry is one provider in the order, under the name it is reported as.
+Entry :: struct {
+ name: string,
+ provider: Provider,
+}
+
+// default_chain is the configured preference. The direct API is tried
+// before the coding assistant, because it can enforce the answer's shape
+// and reports usage; the local gateway is last, as the reading of last
+// resort. REVIEW_MODEL re-points the first slot.
+default_chain :: proc(allocator := context.temp_allocator) -> []Entry {
+ model := os.get_env("REVIEW_MODEL", allocator)
+ if model == "" {
+ model = default_api_model
+ }
+ entries := make([]Entry, 2, allocator)
+ entries[0] = Entry {
+ strings.concatenate({"api/", model}, allocator),
+ Provider{kind = .Api, model = model},
+ }
+ entries[1] = Entry {
+ "pi/maple/glm-5-3-flash",
+ Provider{kind = .Pi, model = "glm-5-3-flash", upstream = "maple"},
+ }
+ return entries
+}
+
+// pick asks every entry a trivial question, in order, and returns the
+// first that answers. Every failure is told through warn, so a review run
+// on the second choice says why the first was passed over, in one line.
+pick :: proc(
+ chain: []Entry,
+ warn: proc(msg: string),
+ allocator := context.allocator,
+) -> (
+ p: Provider,
+ picked: string,
+ err: string,
+) {
+ reasons := make([dynamic]string, context.temp_allocator)
+ for e in chain {
+ answer, ask_err := ask(
+ e.provider,
+ "Answer the user's message.",
+ "Answer with the single word OK.",
+ context.temp_allocator,
+ )
+ if ask_err == "" && strings.trim_space(answer.text) != "" {
+ return e.provider, strings.clone(e.name, allocator), ""
+ }
+ why := ask_err if ask_err != "" else "answered nothing"
+ if i := strings.index_any(why, "\r\n"); i >= 0 {
+ why = why[:i]
+ }
+ warn(fmt.tprintf("skipping %s: %s", e.name, why))
+ append(&reasons, fmt.tprintf("%s: %s", e.name, why))
+ }
+ return {}, "", fmt.aprintf("no provider answered: %s", strings.join(reasons[:], "; ", context.temp_allocator), allocator = allocator)
+}
diff --git a/odin/provider/provider.odin b/odin/provider/provider.odin
@@ -0,0 +1,433 @@
+/*
+Package provider answers a question. What answers it is not this tool's
+business: a console API, a coding assistant on the path, or anything else
+that takes a prompt and returns text. Findings are read out of that text
+by the caller, so every provider answers the same way whether or not it
+can enforce a schema.
+*/
+package provider
+
+import "core:encoding/json"
+import "core:fmt"
+import "core:os"
+import "core:strings"
+import "jfm:sh"
+
+// Answer is what a provider returned, with whatever it could say about
+// the cost. A provider that reports no usage leaves the counts at zero
+// rather than inventing them. replayed marks an answer the cache
+// remembered rather than asked for: it cost nothing this run.
+Answer :: struct {
+ text: string,
+ tokens_in: int,
+ tokens_out: int,
+ cached: int,
+ cost: f64,
+ replayed: bool,
+}
+
+// Kind is which way a provider is asked.
+Kind :: enum {
+ Claude,
+ Pi,
+ Api,
+ Command,
+}
+
+// Provider is one way of asking: the coding assistant claude in its
+// non-interactive mode, the assistant pi through one of its upstreams,
+// the console API directly, or whatever command the environment names.
+Provider :: struct {
+ kind: Kind,
+ model: string,
+ upstream: string,
+ argv: []string,
+ field: string,
+}
+
+// The default models: the middle one rather than the smallest. Measured
+// against the eval set, the smallest reads the shortlist and reports the
+// first duplicate it finds rather than all of them.
+default_claude_model :: "sonnet"
+default_api_model :: "claude-sonnet-5"
+
+// build is the provider called by name, with the model given or its
+// default. The command provider reads its command line from REVIEW_COMMAND
+// and the field its answer arrives in from REVIEW_COMMAND_FIELD.
+build :: proc(which, model: string, allocator := context.allocator) -> (p: Provider, ok: bool) {
+ switch which {
+ case "claude":
+ return Provider{kind = .Claude, model = model if model != "" else default_claude_model},
+ true
+ case "pi":
+ return Provider {
+ kind = .Pi,
+ model = model,
+ upstream = os.get_env("REVIEW_PI_PROVIDER", allocator),
+ },
+ true
+ case "api":
+ return Provider{kind = .Api, model = model if model != "" else default_api_model}, true
+ case "command":
+ return Provider {
+ kind = .Command,
+ argv = strings.fields(os.get_env("REVIEW_COMMAND", allocator), allocator),
+ field = os.get_env("REVIEW_COMMAND_FIELD", allocator),
+ },
+ true
+ }
+ return {}, false
+}
+
+// name is how a provider is asked for, and what the answer cache keys on:
+// it carries the model and the upstream, so that two asks through
+// differently pointed providers are never confused.
+name :: proc(p: Provider, allocator := context.allocator) -> string {
+ switch p.kind {
+ case .Claude:
+ return strings.concatenate({"claude/", p.model}, allocator)
+ case .Pi:
+ parts := make([dynamic]string, context.temp_allocator)
+ append(&parts, "pi")
+ if p.upstream != "" {
+ append(&parts, p.upstream)
+ }
+ if p.model != "" {
+ append(&parts, p.model)
+ }
+ return strings.join(parts[:], "/", allocator)
+ case .Api:
+ return strings.concatenate({"api/", p.model}, allocator)
+ case .Command:
+ if len(p.argv) == 0 {
+ return strings.clone("command", allocator)
+ }
+ return strings.concatenate(
+ {"command: ", strings.join(p.argv, " ", context.temp_allocator)},
+ allocator,
+ )
+ }
+ return ""
+}
+
+// ask puts one question and returns what came back, as text, or why it
+// could not.
+ask :: proc(
+ p: Provider,
+ system, user: string,
+ allocator := context.allocator,
+) -> (
+ Answer,
+ string,
+) {
+ switch p.kind {
+ case .Claude:
+ return ask_claude(p, system, user, allocator)
+ case .Pi:
+ return ask_pi(p, system, user, allocator)
+ case .Api:
+ return ask_api(p, system, user, findings_tool, allocator)
+ case .Command:
+ return ask_command(p, system, user, allocator)
+ }
+ return {}, "no such provider"
+}
+
+// ask_verdict is the second reading's ask: held to its shape where the
+// provider can enforce one, described in the prompt where it cannot.
+ask_verdict :: proc(
+ p: Provider,
+ system, user: string,
+ allocator := context.allocator,
+) -> (
+ Answer,
+ string,
+) {
+ if p.kind == .Api {
+ return ask_api(p, system, user, verdicts_tool, allocator)
+ }
+ return ask(p, system, user, allocator)
+}
+
+// ask_claude asks the coding assistant on the path, in its non-interactive
+// mode, with whatever credentials it already holds. The tools are refused
+// rather than left to judgement: these jobs are given everything they may
+// read, and a reader that goes looking for more is answering a different
+// question from the one asked.
+ask_claude :: proc(
+ p: Provider,
+ system, user: string,
+ allocator := context.allocator,
+) -> (
+ Answer,
+ string,
+) {
+ out, err := shell(
+ {
+ "claude",
+ "-p",
+ "--model",
+ p.model,
+ "--output-format",
+ "json",
+ "--disallowedTools",
+ "Bash,Read,Write,Edit,Glob,Grep,WebFetch,WebSearch,Task,NotebookEdit",
+ "--append-system-prompt",
+ system,
+ },
+ user,
+ context.temp_allocator,
+ )
+ if err != "" {
+ return {}, strings.clone(err, allocator)
+ }
+ Envelope :: struct {
+ result: string `json:"result"`,
+ is_error: bool `json:"is_error"`,
+ cost: f64 `json:"total_cost_usd"`,
+ usage: struct {
+ tokens_in: int `json:"input_tokens"`,
+ tokens_out: int `json:"output_tokens"`,
+ cached: int `json:"cache_read_input_tokens"`,
+ } `json:"usage"`,
+ }
+ envelope: Envelope
+ if json.unmarshal_string(out, &envelope, allocator = context.temp_allocator) != nil {
+ return {}, "reading the answer: not the JSON envelope expected"
+ }
+ if envelope.is_error {
+ return {}, strings.clone(strings.trim_space(envelope.result), allocator)
+ }
+ return Answer {
+ text = strings.clone(envelope.result, allocator),
+ tokens_in = envelope.usage.tokens_in,
+ tokens_out = envelope.usage.tokens_out,
+ cached = envelope.usage.cached,
+ cost = envelope.cost,
+ },
+ ""
+}
+
+// ask_pi asks the assistant of that name, which speaks to several
+// providers of its own; the upstream is named through the environment,
+// since which one is reachable is a property of the machine. Its answer
+// arrives as a stream of events, not as one envelope.
+ask_pi :: proc(
+ p: Provider,
+ system, user: string,
+ allocator := context.allocator,
+) -> (
+ Answer,
+ string,
+) {
+ args := make([dynamic]string, context.temp_allocator)
+ append(
+ &args,
+ "pi",
+ "-p",
+ "--mode",
+ "json",
+ "--no-session",
+ "--no-tools",
+ "--system-prompt",
+ system,
+ )
+ if p.upstream != "" {
+ append(&args, "--provider", p.upstream)
+ }
+ if p.model != "" {
+ append(&args, "--model", p.model)
+ }
+ out, err := shell(args[:], user, context.temp_allocator)
+ if err != "" {
+ return {}, strings.clone(err, allocator)
+ }
+ return spoken(out, allocator)
+}
+
+// Pi_Event is one line of pi's event stream.
+Pi_Event :: struct {
+ type: string `json:"type"`,
+ message: struct {
+ role: string `json:"role"`,
+ content: []struct {
+ type: string `json:"type"`,
+ text: string `json:"text"`,
+ } `json:"content"`,
+ usage: struct {
+ input: int `json:"input"`,
+ output: int `json:"output"`,
+ cache_read: int `json:"cacheRead"`,
+ cost: struct {
+ total: f64 `json:"total"`,
+ } `json:"cost"`,
+ } `json:"usage"`,
+ stop_reason: string `json:"stopReason"`,
+ error_message: string `json:"errorMessage"`,
+ } `json:"message"`,
+}
+
+// spoken reads pi's answer out of its event stream, one JSON object per
+// line. The last assistant message is the answer; everything before it is
+// the working. Whatever pi says outside the stream is the only clue to a
+// refusal, so it is kept for the error.
+spoken :: proc(out: string, allocator := context.allocator) -> (Answer, string) {
+ answer: Answer
+ said := false
+ prose := make([dynamic]string, context.temp_allocator)
+ rest := out
+ for raw in strings.split_lines_iterator(&rest) {
+ line := strings.trim_space(raw)
+ if line == "" {
+ continue
+ }
+ event: Pi_Event
+ if json.unmarshal_string(line, &event, allocator = context.temp_allocator) != nil {
+ append(&prose, line)
+ continue
+ }
+ if event.type != "message_end" || event.message.role != "assistant" {
+ continue
+ }
+ if event.message.stop_reason == "error" {
+ why :=
+ event.message.error_message if event.message.error_message != "" else "the model stopped with an error"
+ return {}, strings.clone(why, allocator)
+ }
+ text := strings.builder_make(allocator)
+ for block in event.message.content {
+ if block.type == "text" {
+ strings.write_string(&text, block.text)
+ }
+ }
+ usage := event.message.usage
+ answer = Answer {
+ text = strings.to_string(text),
+ tokens_in = usage.input,
+ tokens_out = usage.output,
+ cached = usage.cache_read,
+ cost = usage.cost.total,
+ }
+ said = true
+ }
+ if !said {
+ why := first(strings.join(prose[:], " ", context.temp_allocator), 200)
+ return {}, strings.clone(why if why != "" else "no assistant message in the answer", allocator)
+ }
+ return answer, ""
+}
+
+// ask_command asks whatever the environment names, with the prompt on its
+// input, and reads the answer whole or out of the field named.
+ask_command :: proc(
+ p: Provider,
+ system, user: string,
+ allocator := context.allocator,
+) -> (
+ Answer,
+ string,
+) {
+ if len(p.argv) == 0 {
+ return {}, "REVIEW_COMMAND names no command"
+ }
+ out, err := shell(
+ p.argv,
+ strings.concatenate({system, "\n\n", user}, context.temp_allocator),
+ allocator,
+ )
+ if err != "" {
+ return {}, strings.clone(err, allocator)
+ }
+ if p.field == "" {
+ return Answer{text = out}, ""
+ }
+ return Answer{text = field(out, p.field, allocator)}, ""
+}
+
+// shell works a command with the prompt on its input, which every
+// assistant here accepts and which keeps a long prompt out of the
+// argument list. A run past the ask timeout is a failed ask.
+shell :: proc(
+ argv: []string,
+ prompt: string,
+ allocator := context.allocator,
+) -> (
+ out: string,
+ err: string,
+) {
+ full := make([dynamic]string, context.temp_allocator)
+ timed := false
+ if _, found := sh.which("timeout", context.temp_allocator); found {
+ append(&full, "timeout", fmt.tprintf("%d", ask_timeout_seconds()))
+ timed = true
+ }
+ append(&full, ..argv)
+ r := sh.exec(full[:], {stdin = prompt}, allocator)
+ if r.err != nil {
+ return "", fmt.aprintf("%s: %s", argv[0], os.error_string(r.err), allocator = allocator)
+ }
+ if timed && r.code == 124 {
+ return "", fmt.aprintf(
+ "%s: no answer within %d seconds",
+ argv[0],
+ ask_timeout_seconds(),
+ allocator = allocator,
+ )
+ }
+ if !r.ok {
+ detail := strings.trim_space(r.stderr)
+ if detail == "" {
+ detail = strings.trim_space(r.stdout)
+ }
+ if len(detail) > 400 {
+ detail = detail[:400]
+ }
+ return "", fmt.aprintf("%s: exit %d: %s", argv[0], r.code, detail, allocator = allocator)
+ }
+ return r.stdout, ""
+}
+
+// ask_timeout_seconds bounds one ask. A gateway that accepts a
+// reachability ask and then stalls on a real one must not hold the review
+// forever; REVIEW_ASK_TIMEOUT, in seconds, can widen it.
+ask_timeout_seconds :: proc() -> int {
+ if s := os.get_env("REVIEW_ASK_TIMEOUT", context.temp_allocator); s != "" {
+ n := 0
+ for i in 0 ..< len(s) {
+ if s[i] < '0' || s[i] > '9' {
+ n = 0
+ break
+ }
+ n = n * 10 + int(s[i] - '0')
+ }
+ if n > 0 {
+ return n
+ }
+ }
+ return 300
+}
+
+// field pulls the answer out of a JSON envelope, by the name given. An
+// answer that is not JSON at all is returned whole, since plenty of
+// commands simply print what they were asked for.
+field :: proc(out: string, name: string, allocator := context.allocator) -> string {
+ value, err := json.parse_string(out, allocator = context.temp_allocator)
+ if err != nil {
+ return out
+ }
+ if envelope, is_object := value.(json.Object); is_object {
+ if text, is_string := envelope[name].(json.String); is_string && text != "" {
+ return strings.clone(string(text), allocator)
+ }
+ }
+ return out
+}
+
+// first is the start of a string, with an ellipsis where it was cut.
+first :: proc(s: string, n: int) -> string {
+ trimmed := strings.trim_space(s)
+ if len(trimmed) > n {
+ return strings.concatenate({trimmed[:n], "…"}, context.temp_allocator)
+ }
+ return trimmed
+}
diff --git a/odin/provider/provider_test.odin b/odin/provider/provider_test.odin
@@ -0,0 +1,102 @@
+package provider
+
+import "core:strings"
+import "core:testing"
+
+@(test)
+names_carry_the_model :: proc(t: ^testing.T) {
+ testing.expect_value(
+ t,
+ name(Provider{kind = .Claude, model = "sonnet"}, context.temp_allocator),
+ "claude/sonnet",
+ )
+ testing.expect_value(
+ t,
+ name(Provider{kind = .Pi, model = "glm", upstream = "maple"}, context.temp_allocator),
+ "pi/maple/glm",
+ )
+ testing.expect_value(t, name(Provider{kind = .Pi}, context.temp_allocator), "pi")
+ testing.expect_value(
+ t,
+ name(Provider{kind = .Api, model = "claude-sonnet-5"}, context.temp_allocator),
+ "api/claude-sonnet-5",
+ )
+ testing.expect_value(
+ t,
+ name(Provider{kind = .Command, argv = {"echo", "x"}}, context.temp_allocator),
+ "command: echo x",
+ )
+ p, ok := build("api", "", context.temp_allocator)
+ testing.expect(t, ok)
+ testing.expect_value(t, p.model, default_api_model)
+ _, ok = build("nothing", "", context.temp_allocator)
+ testing.expect(t, !ok)
+}
+
+@(test)
+pi_answers_from_its_stream :: proc(t: ^testing.T) {
+ out := `{"type":"message_start"}
+{"type":"message_end","message":{"role":"assistant","content":[{"type":"text","text":"{\"findings\":[]}"}],"usage":{"input":10,"output":2,"cacheRead":1,"cost":{"total":0.5}},"stopReason":"stop"}}
+`
+ answer, err := spoken(out, context.temp_allocator)
+ testing.expect_value(t, err, "")
+ testing.expect_value(t, answer.text, `{"findings":[]}`)
+ testing.expect_value(t, answer.tokens_in, 10)
+ testing.expect_value(t, answer.cost, 0.5)
+ _, err = spoken("Error: no provider\nmore prose\n", context.temp_allocator)
+ testing.expect_value(t, err, "Error: no provider more prose")
+ _, err = spoken(
+ `{"type":"message_end","message":{"role":"assistant","stopReason":"error","errorMessage":"rate limited"}}`,
+ context.temp_allocator,
+ )
+ testing.expect_value(t, err, "rate limited")
+}
+
+@(test)
+command_answers_whole_or_by_field :: proc(t: ^testing.T) {
+ p := Provider {
+ kind = .Command,
+ argv = {"cat"},
+ }
+ answer, err := ask(p, "system", "user", context.temp_allocator)
+ testing.expect_value(t, err, "")
+ testing.expect_value(t, answer.text, "system\n\nuser")
+ fielded := Provider {
+ kind = .Command,
+ argv = {"echo", `{"answer":"hello","other":1}`},
+ field = "answer",
+ }
+ answer, err = ask(fielded, "s", "u", context.temp_allocator)
+ testing.expect_value(t, err, "")
+ testing.expect_value(t, answer.text, "hello")
+ testing.expect_value(t, field("not json", "answer", context.temp_allocator), "not json")
+ _, err = ask(Provider{kind = .Command}, "s", "u", context.temp_allocator)
+ testing.expect(t, strings.contains(err, "names no command"))
+ _, err = ask(Provider{kind = .Command, argv = {"false"}}, "s", "u", context.temp_allocator)
+ testing.expect(t, strings.contains(err, "exit 1"))
+}
+
+@(test)
+api_requests_and_replies_are_shaped :: proc(t: ^testing.T) {
+ body := request(
+ "claude-sonnet-5",
+ "sys \"quoted\"",
+ "user\nline",
+ findings_tool,
+ context.temp_allocator,
+ )
+ for want in ([]string{`"model":"claude-sonnet-5"`, `"text":"sys \"quoted\""`, `"cache_control":{"type":"ephemeral"}`, `"strict":true`, `"name":"report_findings"`, `"text":"user\nline"`, `"temperature":0`}) {
+ testing.expectf(t, strings.contains(body, want), "%s missing from %s", want, body)
+ }
+ answer, err := reply(
+ `{"content":[{"type":"text","text":"hi"},{"type":"tool_use","id":"x","name":"report_findings","input":{"findings":[{"rule":"a","message":"b}"}]}}],"usage":{"input_tokens":5,"output_tokens":3,"cache_read_input_tokens":2}}`,
+ context.temp_allocator,
+ )
+ testing.expect_value(t, err, "")
+ testing.expect_value(t, answer.text, `{"findings":[{"rule":"a","message":"b}"}]}`)
+ testing.expect_value(t, answer.tokens_in, 5)
+ testing.expect_value(t, answer.cached, 2)
+ _, err = reply(`{"content":[{"type":"text","text":"hi"}],"usage":{}}`, context.temp_allocator)
+ testing.expect_value(t, err, "the model answered without reporting")
+ testing.expect_value(t, error_of(`{"error":{"message":"bad key"}}`), "bad key")
+}
diff --git a/odin/report/report.odin b/odin/report/report.odin
@@ -240,9 +240,10 @@ encode :: proc(env: Contract, allocator := context.allocator) -> (out: string, o
}
// compact writes an empty list as [] on one line, where the marshaller
-// leaves a blank line inside it.
+// leaves a blank line inside it, and leaves out a truncation that did not
+// happen, as the Go tool does.
compact :: proc(text: string, allocator := context.allocator) -> string {
- out := text
+ out, _ := strings.replace_all(text, "\n \"truncated\": false,", "", context.temp_allocator)
for depth in 0 ..< 8 {
indent := strings.repeat(" ", depth, context.temp_allocator)
pattern := strings.concatenate({"[\n\n", indent, "]"}, context.temp_allocator)
diff --git a/odin/report/report_test.odin b/odin/report/report_test.odin
@@ -121,6 +121,12 @@ encode_carries_the_contract :: proc(t: ^testing.T) {
testing.expect_value(t, len(root["findings"].(json.Array)), 0)
testing.expect(t, !strings.contains(empty, "null"), "no list is null")
testing.expect(t, strings.contains(empty, `"findings": []`), "an empty list is one line")
+ testing.expect(
+ t,
+ !strings.contains(empty, `"truncated"`),
+ "a truncation that did not happen is left out",
+ )
+ testing.expect(t, strings.contains(out, `"truncated": true`))
}
@(test)
diff --git a/odin/review/main.odin b/odin/review/main.odin
@@ -1,129 +1,306 @@
// review, the Odin reading: what a change adds, read through the language
-// sidecars, judged, and reported. The change is the staged one, or the
-// revision range given.
+// sidecars, measured by the deterministic checks and the repository's
+// analysers, judged by the model jobs, and reported. The change is the
+// staged one, or the revision range given.
//
-// review [--json] [--verbose] [--show] [rev]
+// review [flags] [rev]
// review rules [<rule>]
package main
import "core:fmt"
import "core:os"
+import "core:strings"
import "../analyser"
+import "../cache"
import "../change"
import "../check"
import "../finding"
import "../git"
+import "../job"
+import "../provider"
import "../report"
+import "../reviewer"
import "../tree"
+usage :: `review reads a change the way several narrow readers would.
+
+usage: review [flags] [rev] the staged change, or a revision range
+ review rules [<rule>] the deterministic checks, or one of them
+
+ --json Report findings as JSON, for an agent rather than a person.
+ --verbose Show what each job read and what it cost.
+ --show Print what each job would be sent and stop, without asking anything.
+ --no-verify Skip the second reading that checks what each job reported.
+ --fresh Ask the provider even where the answer cache holds this exact question.
+ --exit-code Exit 1 when a must-fix finding stands, so a hook can refuse the change.
+ --jobs a,b Run only these jobs, comma separated.
+ --provider p Who answers: chain (probe the default order), claude, pi, api, or command.
+ --model m Which model, in whatever form the provider names them.
+`
+
+Flags :: struct {
+ as_json, verbose, show, no_verify, fresh, exit_code: bool,
+ only, which, model, rev: string,
+}
+
main :: proc() {
- as_json, verbose, show: bool
- rev := ""
if len(os.args) > 1 && os.args[1] == "rules" {
rules(os.args[2:])
return
}
- for arg in os.args[1:] {
- switch arg {
- case "--json":
- as_json = true
- case "--verbose":
- verbose = true
- case "--show":
- show = true
- case "-h", "--help":
- fmt.println("usage: review [--json] [--verbose] [--show] [rev]")
- return
+ flags, ok := parse(os.args[1:])
+ if !ok {
+ os.exit(2)
+ }
+ code := run(flags)
+ os.exit(code)
+}
+
+// parse reads the flags. A flag that takes a value takes the next argument
+// or the part after its equals sign.
+parse :: proc(args: []string) -> (flags: Flags, ok: bool) {
+ flags.which = os.get_env("REVIEW_PROVIDER", context.temp_allocator)
+ if flags.which == "" {
+ flags.which = "chain"
+ }
+ flags.model = os.get_env("REVIEW_MODEL", context.temp_allocator)
+ i := 0
+ for i < len(args) {
+ arg := args[i]
+ name, value := arg, ""
+ if eq := strings.index_byte(arg, '='); eq >= 0 && strings.has_prefix(arg, "-") {
+ name, value = arg[:eq], arg[eq + 1:]
+ }
+ take := proc(args: []string, i: ^int, value: string) -> string {
+ if value != "" {
+ return value
+ }
+ if i^ + 1 < len(args) {
+ i^ += 1
+ return args[i^]
+ }
+ return ""
+ }
+ switch name {
+ case "--json", "-json":
+ flags.as_json = true
+ case "--verbose", "-verbose":
+ flags.verbose = true
+ case "--show", "-show":
+ flags.show = true
+ case "--no-verify", "-no-verify":
+ flags.no_verify = true
+ case "--fresh", "-fresh":
+ flags.fresh = true
+ case "--exit-code", "-exit-code":
+ flags.exit_code = true
+ case "--jobs", "-jobs":
+ flags.only = take(args, &i, value)
+ case "--provider", "-provider":
+ flags.which = take(args, &i, value)
+ case "--model", "-model":
+ flags.model = take(args, &i, value)
+ case "-h", "--help", "-help":
+ fmt.print(usage)
+ return flags, false
case:
- rev = arg
+ if strings.has_prefix(arg, "-") {
+ fmt.eprintfln("review: unknown flag %s", arg)
+ fmt.eprint(usage)
+ return flags, false
+ }
+ flags.rev = arg
}
+ i += 1
}
+ return flags, true
+}
+
+// run is the review: gather, read, measure, judge, report. It returns the
+// exit code.
+run :: proc(flags: Flags) -> int {
cwd, cwd_err := os.get_working_directory(context.temp_allocator)
if cwd_err != nil {
fmt.eprintln("review: no working directory")
- os.exit(2)
+ return 2
}
root, in_repo := git.toplevel(cwd)
if !in_repo {
fmt.eprintln("review: not in a git repository")
- os.exit(2)
+ return 2
}
- c, gathered := change.gather(rev, root)
+ c, gathered := change.gather(flags.rev, root)
if !gathered {
- fmt.eprintfln("review: git could not describe %q", rev)
- os.exit(1)
+ fmt.eprintfln("review: git could not describe %q", flags.rev)
+ return 1
}
- if len(c.files) == 0 {
- if as_json {
+ if strings.trim_space(c.diff) == "" {
+ if flags.as_json {
out, _ := report.encode(report.Contract{status = "empty"})
fmt.println(out)
} else {
fmt.println("nothing to review")
}
- return
+ return 0
+ }
+ jobs, job_err := job.chosen(flags.only)
+ if job_err != "" {
+ fmt.eprintfln("review: %s", job_err)
+ return 2
}
- t, at_ok := tree.at(root, rev)
+ t, at_ok := tree.at(root, flags.rev)
if !at_ok {
- fmt.eprintfln("review: could not materialise %q", rev)
- os.exit(1)
+ fmt.eprintfln("review: could not materialise %q", flags.rev)
+ return 1
}
defer tree.close(t)
change.read(&c, t)
c.index, _ = change.index(t)
- if show {
- list(c, t)
- }
+ change.find_candidates(&c)
// The deterministic checks ask nothing of a provider, so they run
// before one is built and survive a model that cannot answer.
+ static := make([dynamic]finding.Finding)
+ append(&static, ..check.run(check.scope_of(&c, t)))
+ _, ranged := tree.ends(flags.rev)
+ append(&static, ..analyser.check(&c, t, ranged))
+
+ if flags.show {
+ for j in jobs {
+ subject := j.subject(&c, context.temp_allocator)
+ fmt.printfln("=== %s ===", j.name)
+ if strings.trim_space(subject) == "" {
+ fmt.print("(nothing to read)\n\n")
+ continue
+ }
+ fmt.printfln("%s", subject)
+ }
+ fmt.println("=== static ===")
+ kept, _ := report.filter(t.dir, static[:])
+ finding.sort(kept)
+ if len(kept) == 0 {
+ fmt.println("(no findings)")
+ }
+ for f in kept {
+ line, _ := strings.replace_all(
+ finding.to_string(f, context.temp_allocator),
+ "\n ",
+ "\n ",
+ context.temp_allocator,
+ )
+ fmt.printfln(" %s", line)
+ }
+ return 0
+ }
+ if flags.verbose {
+ fmt.printfln(
+ "reviewing %d files, %d new names, %d tests, %d comments",
+ len(c.files),
+ len(c.symbols),
+ len(c.tests),
+ len(c.comments),
+ )
+ }
+
+ who, picked, provider_err := choose(flags)
+ if provider_err != "" {
+ fmt.eprintfln("review: %s", provider_err)
+ return 1
+ }
+ if flags.verbose {
+ fmt.printfln("asking %s", picked)
+ }
+ // The cache records what each ask answered, so that a re-run of a
+ // part of the change that did not change replays it instead of
+ // asking. A cache that cannot exist is no fault of the review.
+ answers, _ := cache.open(flags.fresh)
+ defer cache.save(answers)
+ r := reviewer.Reviewer {
+ provider = who,
+ name = provider.name(who),
+ verify = !flags.no_verify,
+ verbose = flags.verbose,
+ cache = answers,
+ }
+ result := reviewer.run(&r, &c, jobs)
+
findings := make([dynamic]finding.Finding)
- append(&findings, ..check.run(check.scope_of(&c, t)))
- _, ranged := tree.ends(rev)
- append(&findings, ..analyser.check(&c, t, ranged))
- finding.sort(findings[:])
+ append(&findings, ..result.findings[:])
+ append(&findings, ..static[:])
kept, dismissed := report.filter(t.dir, findings[:])
+ finding.sort(kept)
+ for &f in kept {
+ if f.file != "" && f.line > 0 {
+ f.snippet = tree.line(t, f.file, f.line)
+ }
+ }
env := report.Contract {
- status = report.status_of(c, 0),
- findings = kept,
- dismissed = dismissed,
+ status = report.status_of(c, len(result.failures)),
+ provider = r.name,
+ findings = kept,
+ retracted = result.retracted[:],
+ failed = faults(result.failures[:]),
+ skipped = result.skipped[:],
uncovered = c.uncovered[:],
+ dismissed = dismissed,
truncated = c.truncated,
+ usage = {
+ tokens_in = result.tokens_in,
+ tokens_out = result.tokens_out,
+ cached = result.cached,
+ replayed = result.replayed,
+ cost = result.cost,
+ },
}
- if as_json {
- out, ok := report.encode(env)
- if !ok {
+ if flags.as_json {
+ out, encoded := report.encode(env)
+ if !encoded {
fmt.eprintln("review: the report could not be encoded")
- os.exit(1)
+ return 1
}
fmt.println(out)
- return
+ } else {
+ for failure in result.failures {
+ fmt.eprintfln(" job failed: %s", failure)
+ }
+ fmt.print(report.render(env, flags.verbose))
+ for gap in c.uncovered {
+ fmt.eprintfln("unread: %s: %s", gap.file, gap.reason)
+ }
}
- fmt.print(report.render(env, verbose))
- for gap in c.uncovered {
- fmt.eprintfln("unread: %s: %s", gap.file, gap.reason)
+ // The review is advisory unless asked to gate: then a must-fix
+ // finding that stands is the one thing it refuses.
+ if flags.exit_code && finding.must_fix(kept) {
+ return 1
}
+ return 0
}
-// list prints what the sidecars read, for a look at the evidence before
-// any judgement of it.
-list :: proc(c: change.Change, t: tree.Tree) {
- added := 0
- for _, lines in c.added {
- added += len(lines)
- }
- fmt.printfln("=== change: %d files, %d added lines ===", len(c.files), added)
- for s in c.symbols {
- mark := " exported" if s.exported else ""
- fmt.printfln("symbol %s:%d %s %s%s", s.file, s.line, s.kind, s.name, mark)
+// choose is the provider the flags name, or the first of the default
+// chain that answers a probe.
+choose :: proc(flags: Flags) -> (who: provider.Provider, picked: string, err: string) {
+ if flags.which == "chain" {
+ return provider.pick(provider.default_chain(), proc(msg: string) {fmt.eprintln(msg)})
}
- for f in c.tests {
- fmt.printfln("test %s:%d %s", f.file, f.line, f.name)
+ built, known := provider.build(flags.which, flags.model)
+ if !known {
+ return {}, "", fmt.aprintf("no provider called %q", flags.which)
}
- for l in c.comments {
- fmt.printfln("comment %s:%d %s", l.file, l.line, l.text)
+ return built, provider.name(built), ""
+}
+
+// faults pairs each failure's job with what failed about it.
+faults :: proc(failures: []string) -> []report.Job_Fault {
+ out := make([]report.Job_Fault, len(failures))
+ for f, i in failures {
+ if sep := strings.index(f, ": "); sep >= 0 {
+ out[i] = report.Job_Fault{f[:sep], f[sep + 2:]}
+ } else {
+ out[i] = report.Job_Fault{f, f}
+ }
}
- fmt.printfln("=== index: %d declarations ===", len(c.index))
+ return out
}
// rules prints the catalogue, or one rule's description, so that an
@@ -132,12 +309,51 @@ list :: proc(c: change.Change, t: tree.Tree) {
rules :: proc(args: []string) {
if len(args) == 0 {
fmt.print(check.catalogue())
+ for j in job.all() {
+ fmt.printfln("\n%s", j.criteria)
+ }
+ return
+ }
+ for j in job.all() {
+ if j.name == args[0] {
+ fmt.print(j.criteria)
+ return
+ }
+ }
+ if j, text, found := criterion(args[0]); found {
+ fmt.printfln("%s, from the %s criteria:\n\n%s", args[0], j, text)
return
}
description, ok := check.describe(args[0])
if !ok {
- fmt.eprintfln("review: no rule called %q", args[0])
+ fmt.eprintfln(
+ "review: no job or rule called %q; the jobs are claims, duplication, hygiene, namer, tests",
+ args[0],
+ )
os.exit(1)
}
fmt.printfln("%s, a deterministic check:\n\n%s", args[0], description)
}
+
+// criterion finds the bullet that defines a rule, in whichever job's
+// criteria holds it, with the lines that continue it.
+criterion :: proc(id: string) -> (name, text: string, found: bool) {
+ marker := strings.concatenate({"- `", id, "`"}, context.temp_allocator)
+ for j in job.all() {
+ lines := strings.split_lines(j.criteria, context.temp_allocator)
+ for line, i in lines {
+ if !strings.has_prefix(line, marker) {
+ continue
+ }
+ kept := make([dynamic]string, context.temp_allocator)
+ for l in lines[i:] {
+ if len(kept) > 0 && !strings.has_prefix(l, " ") {
+ break
+ }
+ append(&kept, l)
+ }
+ return j.name, strings.join(kept[:], "\n", context.temp_allocator), true
+ }
+ }
+ return "", "", false
+}
diff --git a/odin/reviewer/reviewer.odin b/odin/reviewer/reviewer.odin
@@ -0,0 +1,545 @@
+/*
+Package reviewer runs the jobs against a change, through whichever
+provider answers. Each job's findings are put back to the provider once,
+against the same evidence, and the ones a second reading does not let
+stand are retracted rather than reported. The cache records what each ask
+answered, so a re-run of an unchanged part of the change replays instead
+of asking. What every job is told is the Go tool's, word for word, so the
+two tools share one cache.
+*/
+package reviewer
+
+import "base:runtime"
+import "core:encoding/json"
+import "core:fmt"
+import "core:os"
+import "core:strings"
+import "core:sync"
+import "core:thread"
+
+import "../cache"
+import "../change"
+import "../finding"
+import "../job"
+import "../provider"
+import "../report"
+import "../txt"
+
+// instruction is what every job is told, before its own criteria. It is
+// kept identical across jobs and providers, so the only thing that differs
+// between two readings is the criteria and the subject.
+instruction :: `You are reviewing one narrow aspect of a change to a repository.
+
+Report only what the criteria below cover. Everything else is another reader's
+job: say nothing about formatting, style, performance, or correctness unless a
+criterion names it.
+
+Rules for reporting:
+- Every finding cites one rule id from the criteria, exactly as written. A
+ finding citing anything else is discarded.
+- Severity is must-fix when the criterion is plainly broken, consider when it
+ is a judgement call, note otherwise.
+- A finding names the file and line it concerns where it has one.
+- The fix is the concrete change to make, not a restatement of the problem.
+- Reporting nothing is the right answer when the criteria are met. Do not
+ manufacture findings to appear useful.
+- You are shown only part of the change. Never infer what the rest contains.
+
+Answer with one JSON object and nothing else. No preamble, no explanation, no
+code fence:
+
+{"findings":[{"rule":"","severity":"must-fix|consider|note","file":"","line":0,"symbol":"","message":"","fix":""}]}
+
+Report no findings as {"findings":[]}.`
+
+// verify_instruction is what the second reading is told. It sees what
+// the first one saw and nothing the first one concluded beyond the
+// findings themselves, so that a verdict is a reading of the evidence
+// rather than an agreement with a colleague.
+verify_instruction :: `You are checking findings another reviewer made against one narrow aspect of a change.
+
+You are shown the criteria that reader judged against, the same part of the
+change it read, and the findings it reported. For each finding, decide
+whether it holds: the code it points at must meet the fault its rule
+describes, judged from the evidence in front of you.
+
+- Judge every finding, by its number. Report no verdicts but those.
+- holds is false for a finding you would not report yourself from this
+ evidence; say why in reason, in one sentence.
+- A finding that holds but overstates its case does not hold as written.
+
+Answer with one JSON object and nothing else. No preamble, no explanation, no
+code fence:
+
+{"verdicts":[{"index":0,"holds":true,"reason":""}]}`
+
+// again is what a job is told when it answered in prose; verdicts_again
+// the same for the second reading.
+again :: `Your previous answer was not a JSON object, so nothing was read from it.
+Answer again, with the findings you already made, as one JSON object and nothing else.`
+
+verdicts_again :: `Your previous answer was not a JSON object, so nothing was read from it.
+Answer again, with one verdict per finding, as one JSON object and nothing else.`
+
+// Reviewer is how the jobs are asked: the provider, whether to verify,
+// whether to narrate, and the cache to replay from.
+Reviewer :: struct {
+ provider: provider.Provider,
+ name: string,
+ verify: bool,
+ verbose: bool,
+ cache: ^cache.Cache,
+}
+
+// Result is what a set of readings produced. The parts beyond the
+// findings are what a program reading the JSON needs to trust an empty
+// list: which jobs failed, which had nothing to read, and what the
+// asking cost.
+Result :: struct {
+ findings: [dynamic]finding.Finding,
+ failures: [dynamic]string,
+ retracted: [dynamic]report.Retracted,
+ skipped: [dynamic]string,
+ tokens_in: int,
+ tokens_out: int,
+ cached: int,
+ replayed: int,
+ cost: f64,
+}
+
+// Task is one job's reading, as a thread carries it.
+@(private)
+Task :: struct {
+ r: ^Reviewer,
+ j: job.Job,
+ pieces: []^change.Change,
+ result: ^Result,
+ lock: ^sync.Mutex,
+}
+
+// run works every job that has something to read, at once. One job
+// failing does not stop the others: four readings out of five is worth
+// more than none. REVIEW_SERIAL asks them one at a time instead, for
+// providers that cannot take concurrent reads.
+run :: proc(
+ r: ^Reviewer,
+ c: ^change.Change,
+ jobs: []job.Job,
+ serial := false,
+ allocator := context.allocator,
+) -> Result {
+ context.allocator = allocator
+ result := Result {
+ findings = make([dynamic]finding.Finding),
+ failures = make([dynamic]string),
+ retracted = make([dynamic]report.Retracted),
+ skipped = make([dynamic]string),
+ }
+ lock: sync.Mutex
+ // Every job's parts are cut once, and the same cut serves the second
+ // reading, so a verdict is asked against exactly what the finding was
+ // read from.
+ tasks := make([]Task, len(jobs), context.temp_allocator)
+ for j, i in jobs {
+ tasks[i] = Task {
+ r = r,
+ j = j,
+ pieces = job.parts(j, c, allocator),
+ result = &result,
+ lock = &lock,
+ }
+ }
+ one_at_a_time := serial || os.get_env("REVIEW_SERIAL", context.temp_allocator) != ""
+ if one_at_a_time {
+ for &t in tasks {
+ read(&t)
+ }
+ } else {
+ threads := make([dynamic]^thread.Thread, context.temp_allocator)
+ for &t in tasks {
+ ctx := runtime.default_context()
+ ctx.allocator = allocator
+ append(&threads, thread.create_and_start_with_poly_data(&t, read, ctx))
+ }
+ for th in threads {
+ thread.join(th)
+ thread.destroy(th)
+ }
+ }
+ if r.verify {
+ verify(r, tasks, &result, one_at_a_time, allocator)
+ }
+ return result
+}
+
+// read is one job's reading: nothing when there is nothing to read, else
+// each part asked and recorded.
+@(private)
+read :: proc(t: ^Task) {
+ subject := t.j.subject(t.pieces[0], context.temp_allocator)
+ whole := len(t.pieces) == 1
+ if whole && strings.trim_space(subject) == "" {
+ sync.mutex_lock(t.lock)
+ append(&t.result.skipped, strings.clone(t.j.name))
+ sync.mutex_unlock(t.lock)
+ if t.r.verbose {
+ fmt.printfln(" %-12s nothing to read", t.j.name)
+ }
+ return
+ }
+ if t.r.verbose && len(t.pieces) > 1 {
+ fmt.printfln(" %-12s asked in %d parts", t.j.name, len(t.pieces))
+ }
+ for piece, i in t.pieces {
+ found, answer, err := ask(t.r, t.j, t.j.subject(piece, context.temp_allocator))
+ for &f in found {
+ f.part = i
+ }
+ record(t.result, t.lock, t.j.name, found, answer, err)
+ }
+}
+
+// record keeps one ask's findings or failure, and what it cost.
+@(private)
+record :: proc(
+ result: ^Result,
+ lock: ^sync.Mutex,
+ name: string,
+ found: []finding.Finding,
+ answer: provider.Answer,
+ err: string,
+) {
+ sync.mutex_lock(lock)
+ defer sync.mutex_unlock(lock)
+ result.tokens_in += answer.tokens_in
+ result.tokens_out += answer.tokens_out
+ result.cached += answer.cached
+ result.cost += answer.cost
+ if answer.replayed {
+ result.replayed += 1
+ }
+ if err != "" {
+ append(&result.failures, fmt.aprintf("%s: %s", name, err))
+ return
+ }
+ append(&result.findings, ..found)
+}
+
+// ask puts one job's question. The criteria are the system prompt, where
+// a provider that caches anything will cache them; the subject goes last.
+// An answer in prose is asked again once.
+@(private)
+ask :: proc(
+ r: ^Reviewer,
+ j: job.Job,
+ subject: string,
+) -> (
+ found: []finding.Finding,
+ answer: provider.Answer,
+ err: string,
+) {
+ system := strings.concatenate({instruction, "\n\n", j.criteria}, context.temp_allocator)
+ answer, err = answered(r, system, subject, false, job.readable)
+ if err != "" {
+ return nil, {}, err
+ }
+ spent(r, j.name, answer)
+ raw, found_object := txt.object(answer.text)
+ if !found_object {
+ prose := provider.first(answer.text, 200)
+ answer, err = answered(
+ r,
+ system,
+ strings.concatenate({subject, "\n\n", again}, context.temp_allocator),
+ false,
+ job.readable,
+ )
+ if err != "" {
+ return nil, {}, err
+ }
+ spent(r, j.name, answer)
+ raw, found_object = txt.object(answer.text)
+ if !found_object {
+ return nil, answer, fmt.aprintf("no findings object in the answer, twice: %s", prose)
+ }
+ }
+ decoded, ok := job.decode(raw, j.name, job.rules(j.criteria))
+ if !ok {
+ return nil, answer, "reading the answer: the findings are not the shape asked for"
+ }
+ return decoded, answer, ""
+}
+
+// answered asks through the cache: the same question asked of the same
+// provider is replayed rather than asked. Only an answer the caller can
+// read is recorded: a model that spent its whole budget and said nothing
+// would otherwise be replayed on every run.
+@(private)
+answered :: proc(
+ r: ^Reviewer,
+ system, user: string,
+ verdict: bool,
+ readable: proc(text: string) -> bool,
+) -> (
+ provider.Answer,
+ string,
+) {
+ if hit, replayed := cache.get(r.cache, r.name, system, user); replayed {
+ if r.verbose {
+ fmt.printfln(" %-12s replayed", r.name)
+ }
+ return hit, ""
+ }
+ answer: provider.Answer
+ err: string
+ if verdict {
+ answer, err = provider.ask_verdict(r.provider, system, user)
+ } else {
+ answer, err = provider.ask(r.provider, system, user)
+ }
+ if err == "" && readable(answer.text) {
+ cache.put(r.cache, r.name, system, user, answer)
+ }
+ return answer, err
+}
+
+@(private)
+spent :: proc(r: ^Reviewer, name: string, answer: provider.Answer) {
+ if !r.verbose {
+ return
+ }
+ if answer.replayed {
+ fmt.printfln(" %-12s replayed", name)
+ return
+ }
+ fmt.printfln(
+ " %-12s %d in, %d out, %d cached, $%.4f",
+ name,
+ answer.tokens_in,
+ answer.tokens_out,
+ answer.cached,
+ answer.cost,
+ )
+}
+
+// Group is one job's findings from one part of its subject, for the pass
+// that checks them.
+@(private)
+Group :: struct {
+ r: ^Reviewer,
+ j: job.Job,
+ subject: string,
+ indexes: []int,
+ result: ^Result,
+ lock: ^sync.Mutex,
+ drop: ^map[int]bool,
+}
+
+// verify puts each job's findings back to the provider once, against the
+// same evidence the first reading had, and keeps only the ones it also
+// reports. A verdict that cannot be asked fails open: the findings stand,
+// marked unverified, and the failure joins the others. A note is never
+// gated on, so it stands unverified without a second reading.
+@(private)
+verify :: proc(
+ r: ^Reviewer,
+ tasks: []Task,
+ result: ^Result,
+ serial: bool,
+ allocator := context.allocator,
+) {
+ groups := make([dynamic]Group, context.temp_allocator)
+ lock: sync.Mutex
+ drop := make(map[int]bool, context.temp_allocator)
+ for t in tasks {
+ for piece, part in t.pieces {
+ indexes := make([dynamic]int, context.temp_allocator)
+ for f, i in result.findings {
+ if f.job == t.j.name && f.part == part && f.severity != .Note {
+ append(&indexes, i)
+ }
+ }
+ if len(indexes) > 0 {
+ append(
+ &groups,
+ Group {
+ r,
+ t.j,
+ t.j.subject(piece, context.temp_allocator),
+ indexes[:],
+ result,
+ &lock,
+ &drop,
+ },
+ )
+ }
+ }
+ }
+ if len(groups) == 0 {
+ return
+ }
+ if serial {
+ for &g in groups {
+ judge(&g)
+ }
+ } else {
+ threads := make([dynamic]^thread.Thread, context.temp_allocator)
+ for &g in groups {
+ ctx := runtime.default_context()
+ ctx.allocator = allocator
+ append(&threads, thread.create_and_start_with_poly_data(&g, judge, ctx))
+ }
+ for th in threads {
+ thread.join(th)
+ thread.destroy(th)
+ }
+ }
+ // A retracted finding is out of the findings: it is reported as a
+ // retraction, with its reason, not twice over.
+ if len(drop) == 0 {
+ return
+ }
+ survivors := make([dynamic]finding.Finding, allocator)
+ for f, i in result.findings {
+ if !drop[i] {
+ append(&survivors, f)
+ }
+ }
+ result.findings = survivors
+}
+
+// judge asks one group's verdicts and applies them.
+@(private)
+judge :: proc(g: ^Group) {
+ held, answer, err := verdicts(g.r, g.j, g.subject, g.result.findings[:], g.indexes)
+ sync.mutex_lock(g.lock)
+ defer sync.mutex_unlock(g.lock)
+ g.result.tokens_in += answer.tokens_in
+ g.result.tokens_out += answer.tokens_out
+ g.result.cached += answer.cached
+ g.result.cost += answer.cost
+ if answer.replayed {
+ g.result.replayed += 1
+ }
+ if err != "" {
+ append(&g.result.failures, fmt.aprintf("verify/%s: %s", g.j.name, err))
+ for i in g.indexes {
+ g.result.findings[i].verified = false
+ }
+ return
+ }
+ for i, position in g.indexes {
+ if v, judged := held[position]; judged && !v.holds {
+ append(
+ &g.result.retracted,
+ report.Retracted{finding = g.result.findings[i], reason = v.reason},
+ )
+ g.drop[i] = true
+ continue
+ }
+ // A finding the verdict list omits stands rather than falls: a
+ // strict pass would let one dropped number retract everything
+ // the reading found.
+ g.result.findings[i].verified = true
+ }
+}
+
+// Verdict is what the second reading says about one finding.
+Verdict :: struct {
+ holds: bool,
+ reason: string,
+}
+
+// Spoken_Verdicts is the shape the second reading answers in.
+Spoken_Verdicts :: struct {
+ verdicts: []struct {
+ index: int `json:"index"`,
+ holds: bool `json:"holds"`,
+ reason: string `json:"reason"`,
+ } `json:"verdicts"`,
+}
+
+// readable_verdicts is whether an answer holds a verdicts object.
+readable_verdicts :: proc(text: string) -> bool {
+ raw, found := txt.object(text)
+ if !found {
+ return false
+ }
+ read: Spoken_Verdicts
+ return json.unmarshal_string(raw, &read, allocator = context.temp_allocator) == nil
+}
+
+// verdicts puts one job's findings back to the provider, against the same
+// evidence the first reading had. Verdicts are keyed by position in the
+// listing; a position the answer omits stands rather than falls.
+@(private)
+verdicts :: proc(
+ r: ^Reviewer,
+ j: job.Job,
+ subject: string,
+ findings: []finding.Finding,
+ indexes: []int,
+) -> (
+ held: map[int]Verdict,
+ answer: provider.Answer,
+ err: string,
+) {
+ system := strings.concatenate({verify_instruction, "\n\n", j.criteria}, context.temp_allocator)
+ listed := strings.builder_make(context.temp_allocator)
+ for i, position in indexes {
+ f := findings[i]
+ where_at := ""
+ if f.file != "" {
+ where_at = fmt.tprintf(" at %s:%d", f.file, f.line)
+ }
+ fmt.sbprintf(
+ &listed,
+ "%d. [%s] %s%s: %s\n fix: %s\n",
+ position,
+ f.rule,
+ finding.severity_name(f.severity),
+ where_at,
+ f.message,
+ f.fix,
+ )
+ }
+ user := strings.concatenate(
+ {subject, "\n\nThe findings reported against it:\n\n", strings.to_string(listed)},
+ context.temp_allocator,
+ )
+ answer, err = answered(r, system, user, true, readable_verdicts)
+ if err != "" {
+ return nil, answer, err
+ }
+ spent(r, j.name, answer)
+ raised, found := txt.object(answer.text)
+ if !found {
+ prose := provider.first(answer.text, 200)
+ answer, err = answered(
+ r,
+ system,
+ strings.concatenate({user, "\n\n", verdicts_again}, context.temp_allocator),
+ true,
+ readable_verdicts,
+ )
+ if err != "" {
+ return nil, answer, err
+ }
+ spent(r, j.name, answer)
+ raised, found = txt.object(answer.text)
+ if !found {
+ return nil, answer, fmt.aprintf("no verdicts object in the answer, twice: %s", prose)
+ }
+ }
+ read: Spoken_Verdicts
+ if json.unmarshal_string(raised, &read, allocator = context.temp_allocator) != nil {
+ return nil, answer, "the verdicts are not the shape asked for"
+ }
+ held = make(map[int]Verdict, context.temp_allocator)
+ for v in read.verdicts {
+ if v.index >= 0 && v.index < len(indexes) {
+ held[v.index] = Verdict{v.holds, strings.clone(v.reason)}
+ }
+ }
+ return held, answer, ""
+}
diff --git a/odin/reviewer/reviewer_test.odin b/odin/reviewer/reviewer_test.odin
@@ -0,0 +1,143 @@
+package reviewer
+
+import "core:strings"
+import "core:testing"
+
+import "../change"
+import "../job"
+import "../provider"
+
+// echoing is a provider that answers every ask with the text given.
+echoing :: proc(text: string) -> provider.Provider {
+ argv := make([]string, 2, context.temp_allocator)
+ argv[0], argv[1] = "echo", text
+ return provider.Provider{kind = .Command, argv = argv}
+}
+
+with_tests :: proc() -> change.Change {
+ c := change.Change{}
+ c.tests = make([dynamic]change.Function, context.temp_allocator)
+ append(
+ &c.tests,
+ change.Function {
+ name = "TestX",
+ file = "x_test.go",
+ line = 4,
+ body = "func TestX(t *testing.T) {}",
+ },
+ )
+ return c
+}
+
+@(test)
+findings_are_read_and_jobs_without_a_subject_are_skipped :: proc(t: ^testing.T) {
+ context.allocator = context.temp_allocator
+ p := echoing(
+ `{"findings":[{"rule":"cannot-fail","severity":"must-fix","file":"x_test.go","line":4,"symbol":"TestX","message":"asserts nothing","fix":"assert"},{"rule":"invented","severity":"note","message":"x","fix":"y"}]}`,
+ )
+ r := Reviewer {
+ provider = p,
+ name = provider.name(p),
+ verify = false,
+ }
+ c := with_tests()
+ result := run(&r, &c, job.all(), serial = true)
+ testing.expect_value(t, len(result.failures), 0)
+ testing.expect_value(t, len(result.findings), 1)
+ if len(result.findings) == 1 {
+ f := result.findings[0]
+ testing.expect_value(t, f.job, "tests")
+ testing.expect_value(t, f.rule, "cannot-fail")
+ testing.expect_value(t, f.line, 4)
+ testing.expect(t, !f.verified)
+ }
+ testing.expect_value(
+ t,
+ strings.join(result.skipped[:], ",", context.temp_allocator),
+ "duplication,namer,claims,hygiene",
+ )
+}
+
+@(test)
+a_second_reading_retracts_what_it_does_not_hold :: proc(t: ^testing.T) {
+ context.allocator = context.temp_allocator
+ // One answer that is both a findings object and a verdicts object:
+ // the first reading reports one finding, the second says it falls.
+ p := echoing(
+ `{"findings":[{"rule":"cannot-fail","severity":"must-fix","file":"x_test.go","line":4,"symbol":"TestX","message":"m","fix":"f"}],"verdicts":[{"index":0,"holds":false,"reason":"the test can fail"}]}`,
+ )
+ r := Reviewer {
+ provider = p,
+ name = provider.name(p),
+ verify = true,
+ }
+ c := with_tests()
+ result := run(&r, &c, job.all(), serial = true)
+ testing.expect_value(t, len(result.failures), 0)
+ testing.expect_value(t, len(result.findings), 0)
+ testing.expect_value(t, len(result.retracted), 1)
+ if len(result.retracted) == 1 {
+ testing.expect_value(t, result.retracted[0].reason, "the test can fail")
+ testing.expect_value(t, result.retracted[0].finding.rule, "cannot-fail")
+ }
+ // A verdict that holds leaves the finding standing, verified.
+ holds := echoing(
+ `{"findings":[{"rule":"cannot-fail","severity":"must-fix","file":"x_test.go","line":4,"symbol":"TestX","message":"m","fix":"f"}],"verdicts":[{"index":0,"holds":true,"reason":"it stands"}]}`,
+ )
+ r = Reviewer {
+ provider = holds,
+ name = provider.name(holds),
+ verify = true,
+ }
+ c = with_tests()
+ result = run(&r, &c, job.all(), serial = true)
+ testing.expect_value(t, len(result.findings), 1)
+ if len(result.findings) == 1 {
+ testing.expect(t, result.findings[0].verified)
+ }
+}
+
+@(test)
+a_verdict_that_cannot_be_asked_fails_open :: proc(t: ^testing.T) {
+ context.allocator = context.temp_allocator
+ // The answer is findings only, so the verdict ask reads no verdicts
+ // twice over: the findings stand unverified and the failure is named.
+ p := echoing(
+ `{"findings":[{"rule":"cannot-fail","severity":"must-fix","file":"x_test.go","line":4,"symbol":"TestX","message":"m","fix":"f"}]}`,
+ )
+ r := Reviewer {
+ provider = p,
+ name = provider.name(p),
+ verify = true,
+ }
+ c := with_tests()
+ result := run(&r, &c, job.all(), serial = true)
+ testing.expect_value(t, len(result.findings), 1)
+ if len(result.findings) == 1 {
+ testing.expect(t, result.findings[0].verified)
+ }
+ testing.expectf(
+ t,
+ len(result.failures) == 0,
+ "an omitted verdict stands rather than falls: %v",
+ result.failures,
+ )
+ prose := echoing("I have nothing to say")
+ r = Reviewer {
+ provider = prose,
+ name = provider.name(prose),
+ verify = false,
+ }
+ c = with_tests()
+ result = run(&r, &c, job.all(), serial = true)
+ testing.expect_value(t, len(result.failures), 1)
+ if len(result.failures) == 1 {
+ testing.expect(
+ t,
+ strings.has_prefix(
+ result.failures[0],
+ "tests: no findings object in the answer, twice",
+ ),
+ )
+ }
+}
diff --git a/odin/tree/tree.odin b/odin/tree/tree.odin
@@ -116,6 +116,27 @@ sources :: proc(t: Tree, allocator := context.allocator) -> (out: map[string][]b
return out, true
}
+// line is one line of a file at the end of the change, trimmed, or empty
+// where the file or the line is not there.
+line :: proc(t: Tree, name: string, number: int, allocator := context.allocator) -> string {
+ if number < 1 {
+ return ""
+ }
+ data, ok := read(t, name, context.temp_allocator)
+ if !ok {
+ return ""
+ }
+ rest := string(data)
+ n := 0
+ for text in strings.split_lines_iterator(&rest) {
+ n += 1
+ if n == number {
+ return strings.clone(strings.trim_space(text), allocator)
+ }
+ }
+ return ""
+}
+
// exists reports whether the tree holds the path as a file.
exists :: proc(t: Tree, name: string) -> bool {
return os.is_file(path(t, name, context.temp_allocator))
diff --git a/odin/txt/txt.odin b/odin/txt/txt.odin
@@ -0,0 +1,52 @@
+/*
+Package txt is the few readings of plain text that several packages
+share: the words a name is made of, a plural made singular, and the JSON
+object inside an answer.
+*/
+package txt
+
+import "core:strings"
+
+// split_words breaks a name into the words it is made of: a capital
+// starts a word, an underscore ends one, as snake_case languages put the
+// next word after it.
+split_words :: proc(name: string, allocator := context.allocator) -> []string {
+ words := make([dynamic]string, allocator)
+ start := 0
+ for i in 0 ..< len(name) {
+ c := name[i]
+ if i > 0 && ((c >= 'A' && c <= 'Z') || c == '_') {
+ if i > start {
+ append(&words, name[start:i])
+ }
+ start = i + 1 if c == '_' else i
+ }
+ }
+ if start < len(name) {
+ append(&words, name[start:])
+ }
+ return words[:]
+}
+
+// depluralise drops a trailing s, except where dropping it would leave
+// another: class stays class.
+depluralise :: proc(w: string) -> string {
+ if len(w) > 3 && strings.has_suffix(w, "s") && w[len(w) - 2] != 's' {
+ return w[:len(w) - 1]
+ }
+ return w
+}
+
+// object finds the JSON object in a text that has prose around it: from
+// the first brace to the last. Only a provider that can enforce a schema
+// returns bare JSON; the rest wrap it in whatever they were minded to
+// say, and a compiler prints its own prose before its JSON when it cannot
+// even start.
+object :: proc(s: string) -> (string, bool) {
+ start := strings.index_byte(s, '{')
+ end := strings.last_index_byte(s, '}')
+ if start < 0 || end < start {
+ return "", false
+ }
+ return s[start:end + 1], true
+}
