Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ jobs:
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
# Pinned so jj CLI output stays stable for the adapter tests.
- name: Install Jujutsu
run: |
curl -sL -o /tmp/jj.tar.gz https://github.com/jj-vcs/jj/releases/download/v0.44.0/jj-v0.44.0-x86_64-unknown-linux-musl.tar.gz
mkdir -p /tmp/jj-install
tar -xzf /tmp/jj.tar.gz -C /tmp/jj-install
sudo mv /tmp/jj-install/jj /usr/local/bin/jj
jj --version
- name: Configure jj identity
run: |
jj config set --user user.name "Oot CI"
jj config set --user user.email "oot-ci@example.com"
- name: Run Tests
run: cargo test --all-targets --all-features --verbose

Expand All @@ -50,3 +62,24 @@ jobs:
- uses: dtolnay/rust-toolchain@stable
- name: Build Release Binary
run: cargo build --release --verbose

oot:
name: Oot Adjudication (dogfood)
runs-on: ubuntu-latest
if: github.event_name == 'pull_request'
steps:
- uses: actions/checkout@v4
with:
# Full history so git merge-base between the PR branch and main resolves.
fetch-depth: 0
- uses: dtolnay/rust-toolchain@stable
- name: Build Oot
run: cargo build --release
- name: Adjudicate PR against main
run: |
./target/release/oot adjudicate \
--change "${{ github.head_ref }}" \
--base-ref origin/main \
--head-ref HEAD \
--repo . \
--visibility visibility.toml
77 changes: 77 additions & 0 deletions CD_res/implementation/jj-adapter/jj-adapter-research.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# Implementation Research: Multi-Language Structural Engine (Go + JavaScript)

## The Task
Extend oot's structural diff engine (`src/engine/mod.rs`, tree-sitter 0.23, Rust edition implied by Cargo.toml) from Rust-only parsing to also handle Go and JavaScript. Current state:

- `Engine::new()` hard-codes `tree_sitter_rust::LANGUAGE` (src/engine/mod.rs:19)
- Both `diff_snapshots` and `diff_3way` skip any path not ending in `.rs` (lines 37, 130)
- `collect()` matches a single node kind `"function_item"` and reads field `"name"` (line 324) — both are Rust-specific
- Function identity is a bare name string in a flat HashMap — methods in different Rust `impl` blocks already collide today (pre-existing bug that multi-language work will surface)

Constraints respected: adapters are done (git + jj); this touches only the engine layer plus fixtures/tests. YAGNI: no plugin/dynamic-loading systems, no query-file DSL unless it earns its keep.

## 1. Common Gotchas

- **ABI mismatch between grammar crates and the runtime crate.** Each generated parser embeds an ABI version; `tree-sitter` 0.23 accepts ABI 13–14 only. Mixing a grammar crate generated by a newer CLI produces `LanguageError::Version` at `set_language`. Real-world damage report: tree-sitter-rust issue #273 ("Incompatible Language version 15. Must be between 13 and 14") when py-tree-sitter 0.24 met grammar 0.25. — Source: https://github.com/tree-sitter/tree-sitter-rust/issues/273 ; ABI table: https://tree-sitter-tree-sitter.mintlify.app/using-parsers/abi-versions ("≥ 0.20.3, ≤ 0.24 → 13 | 14"). Avoidance: take all three grammar crates (`tree-sitter-rust`, `tree-sitter-go`, `tree-sitter-javascript`) from the same 0.23-generation release train and add a unit test that calls `set_language` for every supported language at startup so CI fails loudly on drift. Note `set_language` returns `Result` — mismatch is a graceful error, not a panic (docs.rs/tree_sitter Parser::set_language).
- **Grammar crate versions are decoupled from the runtime crate version.** `tree-sitter-rust 0.23.0` depends on `tree-sitter-language ^0.1` and is developed against `tree-sitter ^0.23` (docs.rs/crate/tree-sitter-rust/0.23.0), but e.g. `tree-sitter-go`'s latest tags (v0.21.x era shown in release diffs) do not necessarily track the same number. Do not assume "same number = compatible"; assume "must verify via set_language in tests."
- **JavaScript functions are frequently anonymous.** From node-types.json (master): `arrow_function` has fields only `body`/`parameter`/`parameters` — no `name`; `function_expression` and `generator_function` have optional `name`. Named extraction must fall back to context (the enclosing `variable_declarator`, `assignment_expression` left side, or `method_definition` property) or those nodes are silently dropped. — Source: https://raw.githubusercontent.com/tree-sitter/tree-sitter-javascript/master/src/node-types.json
- **JS functions hide behind wrappers.** `export_statement` wraps declarations (`fields.declaration`); class methods are `method_definition` inside `class_body` (node-types.json confirms both). A shallow top-level-only scan misses most real-world JS. The existing recursive `collect()` walker survives this unchanged — good news.
- **Parse failures are silent.** Tree-sitter is error-tolerant: a file with syntax errors still yields a tree containing `ERROR` nodes, and `extract_functions` will happily index garbage fragments as "functions". No current source documents an ERROR check in oot. Avoidance: after parse, walk once for `ERROR`/`MISSING` on the root's immediate children; if found, emit a Review-severity dispute ("could not fully parse") instead of trusting extracted names.
- **Binary size creep.** ast-grep compiles each parser behind a Cargo feature flag precisely because grammars are large C files. — Source: https://deepwiki.com/ast-grep/ast-grep/4-language-support ("Languages are conditionally compiled using Cargo features to reduce binary size"). For three languages this is optional; flag it, don't build it yet (YAGNI).

## 2. Best Practices

- **Per-language config table over scattered conditionals.** ast-grep's core abstraction is a `Language` trait exposing `from_path` (extension → language detection), `kind_to_id`, and `field_to_id` — everything downstream is written against the trait, never against concrete grammars. — Source: https://docs.rs/ast-grep-language/latest/ast_grep_language/trait.Language.html . Idiomatic translation for oot at our scale: a small `Lang` enum with a static table: `{ extensions, ts_language fn, function_kinds: &[&str], name_field_strategy }`. This replaces both `.ends_with(".rs")` filters and the hardcoded kind string.
- **Extension-based detection is the industry norm.** ast-grep's `SupportLang` enum carries aliases + file extensions for automatic detection (DeepWiki, same page). GitHub linguist does the same. Nothing fancier is warranted for v1.
- **Reuse one `Parser`, switch languages per file** OR hold one parser per language. `set_language` is designed for exactly this and validates ABI each call (docs.rs). Per-language parser construction in `Engine::new()` (a HashMap<Lang, Parser>) avoids repeated validation and keeps `diff_snapshots`' hot loop clean. Either is defensible; per-parser-map is the simpler mental model.
- **Node-kind sets, not single kinds.** Rust: just `function_item`. Go: `function_declaration` AND `method_declaration` (both carry field `name` — verified in src/grammar.json of tree-sitter-go). JS: `function_declaration`, `function_expression`, `generator_function_declaration`, `generator_function`, `method_definition` (all named-node kinds from node-types.json). A `&[&str]` per language covers this without a query DSL.

## 3. Pitfalls & Language Quirks

- **Method-name collision semantics differ per language and are currently wrong-ish for Rust too.** Today, `impl Foo { fn run }` and `impl Bar { fn run }` map to the same key `"run"` — a false 3-way conflict. In Go, `method_declaration`'s name field is a `_field_identifier` scoped to a receiver (grammar.json); correct identity is `Type.name` (receiver type from the `receiver` field). In JS, `method_definition` names can be `computed_property_name` or string literals (node-types.json) — stringify what's there, skip truly dynamic ones. Decision needed: minimum viable fix is prefixing Rust method names with their enclosing `impl` type and Go ones with the receiver type; JS `method_definition` gets its property text.
- **Arrow functions assigned to variables:** `const f = () => {}` puts the name on the `variable_declarator`, not the `arrow_function`. v1 call: track only named forms + variable-assigned arrows/functions via parent lookup one level up; anything deeper (object literal properties, default exports) waits until there's a failing case (YAGNI).
- **`.rs`-style filtering duplicated in two methods** (lines 37 and 130): replace both with the same `Lang::from_path(path)` helper or the two will drift.
- **Row numbers:** `start_position().row + 1` is already correct and language-independent; byte-offset `utf8_text` handles UTF-8 fine. No change needed.
- **Version note:** findings apply to `tree-sitter = "0.23"` with same-generation grammar crates (Rust 0.23.x; Go/JS pinned to releases whose generated ABI ∈ [13,14] — exact crate versions to be locked by the CI set_language test, not by assumption).

## 4. Differentiation

- **Industry standard:** ast-grep/semgrep-class tools ship dozens of languages behind feature flags with trait-based language abstraction, query DSLs, and dynamic loading.
- **Our approach:** three languages, one static table, no DSL, no dynamic loading, no features. Function-level identity with receiver-aware naming (which even some diff tools get wrong).
- **Is the difference useful?** Yes-with-a-caveat. The difference is *restraint*, which serves oot's actual job (adjudicate changes, not search code). The genuinely differentiated piece is folding method receivers into function identity — that fixes a live correctness bug in the 3-way conflict detector, not just adds languages. If we later need 10+ languages, copy ast-grep's feature-flag model then.

## Recommendation

Build in this order:

1. `Lang` enum + static config table (extensions, grammar, function kinds) in the engine module; `Engine::new()` builds a parser per language; CI/unit test asserts `set_language` succeeds for all three (ABI tripwire).
2. Replace both `.rs` filters with `Lang::from_path`; unknown extensions keep being skipped silently.
3. Generalize `collect()`: match any kind in the language's function-kinds list; read `name` field; add receiver-prefix logic (Go `Type.name`, Rust `ImplType::name`, JS `method_definition` property).
4. Parse-health check: root-level `ERROR` scan → Review dispute instead of silent garbage.
5. Fixtures + integration tests: Go fixture pair (func + method rename across branches) and JS fixture pair (named function + const-arrow), driving both `diff_snapshots` and `diff_3way`.
6. README status checkboxes updated.

Explicitly out of scope: TSX/TS, Python, feature flags, query DSL, anonymous-function heuristics beyond one-level parent lookup.

## Sources
- https://github.com/tree-sitter/tree-sitter-rust/issues/273 (ABI mismatch failure mode)
- https://tree-sitter-tree-sitter.mintlify.app/using-parsers/abi-versions (ABI compat table, 0.23 ↔ ABI 13–14)
- https://docs.rs/tree-sitter/latest/tree_sitter/struct.Parser.html (set_language contract)
- https://docs.rs/tree-sitter/latest/tree_sitter/constant.LANGUAGE_VERSION.html
- https://docs.rs/crate/tree-sitter-rust/0.23.0 (grammar↔runtime dependency shape)
- https://github.com/tree-sitter/tree-sitter-go + src/grammar.json (function_declaration/method_declaration, name fields)
- https://docs.rs/tree-sitter-go/latest/tree_sitter_go/constant.NODE_TYPES.html
- https://raw.githubusercontent.com/tree-sitter/tree-sitter-javascript/master/src/node-types.json (JS kinds, anonymous functions, export_statement wrapping)
- https://deepwiki.com/ast-grep/ast-grep/4-language-support (feature flags, extension detection)
- https://docs.rs/ast-grep-language/latest/ast_grep_language/trait.Language.html (Language trait / from_path pattern)
- https://ast-grep.github.io/advanced/core-concepts (kinds vs fields)

## Adversarial Verification

Findings from three adversarial passes and how the plan changed:

1. **Receiver-prefixed method identity → CUT.** Prefixing (`Type.name`) fixes today's bare-name collision but breaks key stability: moving a fn between impl blocks or renaming an impl turns a no-op into remove+add dispute pairs, and in `diff_3way` can produce a false `Severity::High` conflict that flips the verdict to Blocked under default `block_on: ["high"]` policy (src/policy.rs:21). Also unhandled: Go pointer receivers (`(t *T)` vs `(t T)`), Rust trait-impl ambiguity, generic type text instability, JS multi-class same-method-name. Verdict: trades one false-positive class for a worse one on the blocking path. Bare names stay; collision documented as known limitation.
2. **ERROR-node scan → CUT to future work.** Mechanically fine but contradicts the tested graceful-degradation contract (`test_engine_syntax_error_handling`, tests/engine_test.rs:176): tree-sitter recovery already handles garbage input, so an ERROR scan double-reports the same root cause and pollutes dockets for vendored/generated files with stray ERROR nodes.
3. **Exact-match test pins survive** only if free functions keep bare names — confirmed by review of tests/engine_test.rs:39,81,88 and cli_test.rs live-engine assertions. With prefixing cut, this is moot but recorded as the reason it would have broken.
4. **Scope confirmed:** all Rust-only assumptions live in engine/mod.rs only (language field :13, :19; `.rs` filters :37, :130; kind string :324). Adapters/policy/docket consume disputes generically. Only stale item: `test_engine_non_rust_file_filtering` premise wording.
5. **Crate claims verified against crates.io/docs.rs (live):** tree-sitter-go has 0.23.0–0.23.4 releases; tree-sitter-javascript has 0.23.0–0.23.1; both export `LANGUAGE: LanguageFn`; pinned docs.rs/tree-sitter/0.23.2 shows LANGUAGE_VERSION=14 / MIN_COMPATIBLE_LANGUAGE_VERSION=13. Pinning `tree-sitter-go = "0.23"`, `tree-sitter-javascript = "0.23"` alongside the existing runtime 0.23 is safe.

Status: GREEN (issues found were fixed by cutting items 1–2 from scope rather than reworking them).

## Post-Review Round (interrogate, 4 reviewers)

A second adversarial pass on the implemented branch found four consensus issues, all fixed in the follow-up commit:

1. **Arrow/anonymous JS functions invisible** — the "variable-assigned via parent lookup" scoped in this doc's §Recommendation was dropped during implementation without being recorded as a cut. Fixed: `arrow_function` added to kinds; anonymous matches inherit the enclosing `variable_declarator`'s name.
2. **`from_path` dotless regression** — `rsplit('.').next()` matched extension-less files named literally `go`/`js`/`rs` (the old `.ends_with(".rs")` required a dot). Fixed with `rsplit_once('.')` + lowercase normalization.
3. **Silent unreachable guards** — `get_mut(&lang) else { continue }` could have masked future registration drift. Dissolved entirely by restructuring.
4. **Bare-name collision = silent data loss, not noise** — last-wins insert made diffs direction-dependent (editing the first same-named method produced zero disputes). Fixed: first occurrence wins and an explicit Review-severity ambiguity dispute is emitted.

Also applied from reviewer pushback: Engine stores `tree_sitter::Language` (validated in `new()`) instead of live `Parser`s, restoring `&self` on both diff methods and reverting the `&mut` ripple through adapters/main/tests; dispute numbering now sorts function names in `diff_snapshots` (matching `diff_3way`) so saved dockets are deterministic; `collect` no longer recurses into matched functions (kills nested double-reporting); README documents the ambiguity limitation. Note for the record: one reviewer claimed `Language` is `Copy` — it is not; it borrows.
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ toml = "0.8"
anyhow = "1"
tree-sitter = "0.23"
tree-sitter-rust = "0.23"
tree-sitter-python = "0.23"
tree-sitter-javascript = "0.23"
tree-sitter-go = "0.23"
tree-sitter-javascript = "0.23"
tree-sitter-python = "0.23"
38 changes: 26 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,28 +63,42 @@ If a dispute crosses policy, Oot blocks the change or cloaks the private parts.

## Status

We are early. The current code is a seed: a Rust structural-diff engine, a dispute type, a policy loader, and a docket format. None of it yet speaks the Change model end to end. The original five minute pitch was a semantic merge-conflict checker. We are building the governance platform instead, so the build order leads with visibility:
Working seed — the engine runs, the docket renders, and git + Jujutsu ingestion are in-memory. Current focus: using Oot to govern Oot's own changes.

- [ ] Change ingestion from git and Jujutsu snapshots
- [ ] Visibility policy: private paths, private branches, embargo schedules (the governance spine)
- [ ] Meaning disputes from the structural engine plus a hosted intent check
- [ ] Docket format with visibility and embargo state
- [ ] In-memory execution path (no materialized tree)
- [ ] git and Jujutsu adapters, plus a hosted model API for intent
- [x] Change ingestion from git snapshots (in-memory via `git ls-tree`/`cat-file`) and materialized dirs
- [x] Jujutsu ingestion (in-memory via `jj file list`/`file show`, revset resolution, first-class conflict detection)
- [x] Visibility policy: private paths, private branches, embargo schedules (the governance spine)
- [x] Meaning disputes from the structural engine (tree-sitter: Rust, Go, JavaScript)
- [x] Docket format with visibility and embargo state (JSON/TOML + render)
- [x] In-memory execution path (no materialized tree required for git)
- [x] Git adapter with 3-way adjudication
- [x] Jujutsu adapter with 3-way adjudication (`--source jj`, revsets accepted)

## Open source and the model
**Known limitation:** functions that share a bare name within one file (e.g., a `render` method on two classes, or same-named Go methods on two types) are tracked by first occurrence only; the docket flags them as ambiguous rather than tracking each definition separately.

The adjudication runtime, the docket format, and the adapters are MIT licensed. The hosted model that scores intent and runs embargo distribution will be a paid service. A court that hides its deliberations is not a court, so the gate stays open.
## License

## Try it
The adjudication runtime, the docket format, and the adapters are MIT licensed. A court that hides its deliberations is not a court, so the gate stays open.

## Someday

Deliberately unbuilt. These need users to be worth their cost, and there are none yet.

The runtime is not buildable to this shape yet. When it is:
- **Hosted intent scoring** — a model that checks what a change claims to mean against what it actually does. The structural engine catches *that* code changed; this would catch *what it means*. Needs a server, a model, and someone paying for both.
- **Embargo distribution** — the courier half of embargo: quietly shipping held patches to maintainers before the public diff drops. Needs keyed private channels and maintainer auth. The detection half already ships.

## Try it

```bash
git clone https://github.com/Epoch-AI-Lab/oot.git
cd oot
cargo build --release
./target/release/oot adjudicate --change feature/auth-refactor
# materialized dirs
./target/release/oot adjudicate --change feature/auth-refactor --base fixtures/repo/base --head fixtures/repo/head --visibility fixtures/visibility.toml
# or 3-way git (in-memory, no checkout)
./target/release/oot adjudicate --change feature/auth-refactor --base-ref main --head-ref feature/auth --repo .
# or 3-way jujutsu (revsets welcome)
./target/release/oot adjudicate --source jj --change greet --base-ref 'bookmarks(exact:main)' --head-ref '@-'
```

## Contribute
Expand Down
11 changes: 11 additions & 0 deletions TODO.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Known friction

## ~~Fixture `.env` policy noise~~ RESOLVED 2026-08-21

Originally `VisibilityPolicy::check` flagged private-path fragments against
*every* file in the head snapshot, so the intentional `.env` fixture cloaked
every change. Fixed by aligning the code with its own documented contract:
only paths *touched* by a change (added, removed, or content-modified vs
base) are checked. See `test_visibility_policy_only_flags_touched_private_paths`.

No open items.
Loading
Loading