test(analysis): paired silent/firing fixtures for every analyzer rule (#81 P0) - #109
Merged
Merged
Conversation
arena-ai-coding-agent
Bot
requested a review
from hyperpolymath
as a code owner
September 26, 2026 00:06
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Issue #81, P0: "Give every analyzer rule a paired silent/firing fixture; malformed or unsupported input must report 'no check performed' rather than green." This lands that item. crates/oikosbot-analysis/tests/pattern_fixtures.rs covers all seven rules in patterns::detect_patterns. Each pair differs in exactly the feature that triggers the rule under test, so the silent side proves the silence is for the intended reason rather than because the fixture happens to be clean: nested-loops depth 3 vs depth 2 busy-wait spin vs spin-with-one-sleep-per-iteration string-concat-in-loop s = s + "-" vs push_str clone-in-loop item.clone().len() vs item.len() unbuffered-io File::open vs BufReader::new(File::open(..)) large-allocation with_capacity(2_000_000) vs with_capacity(1024) redundant-allocation five .to_string() calls vs four Several pairs deliberately pin a boundary rather than the mere presence of a pattern: nested-loops at the depth-3 threshold, redundant-allocation at the five-call threshold, large-allocation at the 1 MB threshold. If a threshold moves, these fail. Every fixture is a single function and asserts the published AnalysisResult:: rule_id, not the private detectors, per #81's "tests assert observable behaviour". The single-function constraint is load-bearing: rule_id reports the most significant pattern in detect_patterns order, so a fixture tripping two rules would only prove the first. The pairs therefore double as a check that the rules do not fire spuriously on each other's inputs. The "no check performed" cases assert the acceptance rule that missing tools, unparsed inputs and crashes are not passes: a source with no functions, malformed source and truncated source each yield zero findings rather than a green oikosbot/general, and an unsupported extension is an error naming the reason. Verification (no Rust toolchain in this sandbox): all 17 sources in the test file were run through a faithful port of detect_patterns against the tree-sitter-rust grammar and produce exactly the verdicts asserted — 14 paired fixtures plus the three no-check cases, 0 divergences. tools/ci/linter-verify.sh passes all four steps. CI on the merged #48 work already proved the workspace builds and tests green; this file is additive and needs the same confirmation. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
arena-ai-coding-agent
Bot
force-pushed
the
arena/01a0dad6-oikosbot
branch
from
September 26, 2026 00:06
6650781 to
ab0222d
Compare
docs/handoff/owner-actions.adoc collects everything that needs a permission a
checkout does not have, so the handoff does not have to be reconstructed from a
session log:
* the Actions actor policy that makes every pull_request workflow on an agent
branch fail at startup ("Actor is not allowed to trigger Actions workflows"),
with the three ways past it — re-run the workflows as the owner, allow the
bot actor in Actions settings, or merge to main;
* the four phantom required contexts in the main ruleset, why each can never
report, and the reproduction command — including the point that protection
which can never be satisfied trains everyone into --admin, which bypasses all
27 checks including the 23 real ones;
* the design rulings the code is waiting on: the EcoScore scale (now that
calibrated energies saturate calculate_eco_score near 100 and the
eco-threshold gate is near-vacuous), the never-executed Datalog engine, the
Eclexia fake gate, #12's family-4 mapping, and three minor open questions;
* ready-to-post issue comments, since the automation cannot comment on or close
issues ("Resource not accessible by integration");
* the other repositories: the consumer fleet that needs its own lockfiles, and
the repo-wide startup failure in metadatastician/idaptik-ums.
docs/README.adoc indexes it under a new Handoff section.
DEBT.adoc's newly resolved entries are re-wrapped to the file's prevailing
style — they had been written as single long lines running to 895 characters,
against a file whose cells wrap near 80. The Summary table keeps its
single-line-row convention, which is what it already used.
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
hyperpolymath
approved these changes
Sep 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Issue #81, P0 — "Give every analyzer rule a paired silent/firing fixture;
malformed or unsupported input must report 'no check performed' rather than
green."
crates/oikosbot-analysis/tests/pattern_fixtures.rscovers all seven rules inpatterns::detect_patterns. Each pair differs in exactly the feature thattriggers the rule under test:
nested-loopsbusy-waitloopthat never blocksstring-concat-in-loopout = out + "-"out.push_str("-")clone-in-loopitem.clone().len()item.len()unbuffered-ioFile::open(path)?BufReader::new(File::open(path)?)large-allocationwith_capacity(2_000_000)with_capacity(1024)redundant-allocation.to_string()callsThree pairs deliberately pin a boundary rather than the mere presence of a
pattern — depth 3, the 1 MB threshold, the five-call threshold — so a threshold
change breaks the test instead of silently redefining the rule.
Two design constraints worth flagging
Every fixture is a single function.
AnalysisResult::rule_idreports themost significant pattern in
detect_patternsorder, so a fixture that trippedtwo rules would only prove the first one. The single-function constraint turns
each pair into a check that the rules do not fire spuriously on each other's
inputs as well.
The tests assert the published
rule_id, not the private detectors, per#81's "tests assert observable behaviour, not private implementation
details." No filesystem is needed:
analyze_sourcetakes the source directly."No check performed" is not a pass
notes.txt) → an error naming the reasonNone of these may come back as a green
oikosbot/general.Verification
No Rust toolchain exists in this sandbox (rustup and crates.io are both blocked
by the proxy allowlist), so all 17 sources in the test file were run through a
faithful port of
detect_patterns— loop-depth counting, the busy-waitsubstring exclusions,
find_in_loop_body, the unbuffered-I/O parent check, thelarge-allocation digit scan, and the redundant-allocation counter — against the
tree-sitter-rust grammar:
tools/ci/linter-verify.sh→ PASS on all four steps (SPDX headers,permissions declaration, actions lockfile coverage,
lockcheck.sh).and no line exceeds rustfmt's default 100-column width.
tree-sitter-rustgrammar, while the workspacepins crate
tree-sitter-rust0.24.2. The fixtures only rely on node kindsstable across both (
function_item,for_expression,while_expression,loop_expression,binary_expression,call_expression,method_call_expression), and every detection is text-based on the functionor loop body, so the risk is low — but CI remains the authority.
Also in this commit:
DEBT.adoc's licence-identifier count moves 159 → 160 forthe new file, so the register's own claim stays reproducible.
Additive only — no production code changes.