feat(spo): Foundry-shape emitters — codegen_spine + odoo_ontology + action_emitter + link_chain#432
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR implements the four core Opus 4.8 deliverables: foundational codegen spine contracts for ontology-driven pipelines, an Odoo ontology loader, a deterministic action specification projection, and a dependency chain decomposition module. All modules include extensive unit tests validating correctness, determinism, and integration with the embedded ontology fixture. ChangesOntology-Driven Action and Link Chain Emitters with Core Codegen Contracts
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0dfd6014c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Truth-value tolerance check. | ||
| let tol = P::truth_tolerance(); | ||
| if tol > 0.0 { |
There was a problem hiding this comment.
Here the default truth_tolerance() is 0.0, which the trait documents as lossless, but this guard skips the truth comparison entirely for that default. Any projection that preserves (s,p,o) keys while decompiling arbitrary f/c values will incorrectly pass roundtrip_eq; only implementors that opt into a positive tolerance are checked. Please run the truth comparison for zero tolerance as an exact check instead of disabling it.
Useful? React with 👍 / 👎.
AdaWorldAPI
pushed a commit
that referenced
this pull request
May 29, 2026
…tology Pure rustfmt output (cargo fmt -p lance-graph). Line-wrapping preferences (collapse 4-line .get/.unwrap_or chains, expand single-line function signatures past 100 cols). No semantic changes. Fixes the `format` CI check on PR #432.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/knowledge/foundry-workshop-elixir-rust-evaluation.md:
- Around line 24-36: The fenced diagram block in the markdown lacks a language
tag causing markdownlint MD040; update the triple-backtick fence around the
ASCII diagram in .claude/knowledge/foundry-workshop-elixir-rust-evaluation.md to
include a language (e.g., add "text" after ```), ensuring the opening fence
becomes ```text so the diagram is properly annotated for linting.
In @.claude/knowledge/semantic-operational-handbook-v0.1.md:
- Around line 22-29: Many fenced code blocks in
.claude/knowledge/semantic-operational-handbook-v0.1.md are unlabeled causing
MD040 warnings; go through the document and add appropriate language identifiers
(e.g., text, json, elixir, bash) to each triple-backtick block so linters can
parse them. Locate unlabeled fences near headings like "Ontology" and elsewhere,
pick the most specific language for the content in each block (use text for
plain examples), and update all ``` blocks consistently to ```text, ```json,
```elixir, ```bash, etc., ensuring no unlabeled fences remain.
In `@crates/lance-graph-contract/src/codegen_spine.rs`:
- Around line 159-179: The truth-value check is skipped when
P::truth_tolerance() == 0.0 due to the `if tol > 0.0` guard; to fix, remove that
conditional so the comparison loop always runs and keep the existing comparison
using `tol` (i.e., use `(r.f - f0).abs() > tol || (r.c - c0).abs() > tol`) so
that a zero tolerance enforces exact equality; update the block around `let tol
= P::truth_tolerance()` and the loop over `regenerated` (which builds `in_truth`
from `input` and constructs the `RoundTripFailure` with `P::name()`) to always
perform the check.
In `@crates/lance-graph/src/graph/spo/odoo_ontology.rs`:
- Around line 73-101: Change parse_triples to return Result<Vec<OntologyTriple>,
Error> and make load_ontology return Result<SpoStore, Error>, surfacing JSON
parse errors instead of silently discarding them; specifically replace the
.filter_map(... .ok()) in parse_triples with explicit per-line
serde_json::from_str error handling that yields a descriptive snafu error
(include the offending line and serde error) and propagate that Result up to
load_ontology so it fails fast; keep using label_fp, SpoBuilder::build_edge,
dn_hash, TruthValue::new, and SpoStore::insert but call them only after
successful parsing, and add appropriate snafu error variants and context per the
crate's error patterns to identify malformed NDJSON lines and I/O/format issues.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 52a26cc9-fd5e-4335-9f0e-7fda7e1de373
📒 Files selected for processing (10)
.claude/board/AGENT_LOG.md.claude/knowledge/foundry-workshop-elixir-rust-evaluation.md.claude/knowledge/semantic-operational-handbook-v0.1.mdcrates/lance-graph-contract/src/codegen_spine.rscrates/lance-graph-contract/src/lib.rscrates/lance-graph/src/graph/spo/action_emitter.rscrates/lance-graph/src/graph/spo/link_chain.rscrates/lance-graph/src/graph/spo/mod.rscrates/lance-graph/src/graph/spo/odoo_ontology.rscrates/lance-graph/src/graph/spo/odoo_ontology.spo.ndjson
Comment on lines
+24
to
+36
| ``` | ||
| Workshop low-code surface (what a domain expert authors) | ||
| │ view/action/widget bound to ontology | ||
| ▼ | ||
| Elixir-syntax abstraction (readable DSL — NOT BEAM at runtime) | ||
| │ tree-sitter-elixir parse → typed resolve against ontology | ||
| ▼ | ||
| Rust compiled underneath (the holy grail — executable semantics) | ||
| │ recipe → KernelHandle → ractor mailbox → SPO store | ||
| ▼ | ||
| SPO ontology substrate (what names resolve against) | ||
| odoo_ontology.spo.ndjson → SpoStore (22 245 triples, VERIFIED) | ||
| ``` |
There was a problem hiding this comment.
Specify a fenced code language to satisfy markdownlint MD040.
The diagram fence is unlabeled, which triggers lint warnings. Please annotate it as text (or another appropriate language).
💡 Suggested change
-```
+```text
Workshop low-code surface (what a domain expert authors)
│ view/action/widget bound to ontology
▼
Elixir-syntax abstraction (readable DSL — NOT BEAM at runtime)
│ tree-sitter-elixir parse → typed resolve against ontology
▼
Rust compiled underneath (the holy grail — executable semantics)
│ recipe → KernelHandle → ractor mailbox → SPO store
▼
SPO ontology substrate (what names resolve against)
odoo_ontology.spo.ndjson → SpoStore (22 245 triples, VERIFIED)</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/knowledge/foundry-workshop-elixir-rust-evaluation.md around lines 24
- 36, The fenced diagram block in the markdown lacks a language tag causing
markdownlint MD040; update the triple-backtick fence around the ASCII diagram in
.claude/knowledge/foundry-workshop-elixir-rust-evaluation.md to include a
language (e.g., add "text" after ```), ensuring the opening fence becomes
```text so the diagram is properly annotated for linting.
Comment on lines
+22
to
+29
| ``` | ||
| Ontology | ||
| → Objects | ||
| → Links | ||
| → Actions | ||
| → Functions | ||
| → Workshop / Slate / Quiver / Object Explorer / Automate | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks across this document.
This file has many unlabeled code fences, which triggers repeated MD040 warnings. Please label blocks (text, elixir, json, etc.) consistently.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/knowledge/semantic-operational-handbook-v0.1.md around lines 22 -
29, Many fenced code blocks in
.claude/knowledge/semantic-operational-handbook-v0.1.md are unlabeled causing
MD040 warnings; go through the document and add appropriate language identifiers
(e.g., text, json, elixir, bash) to each triple-backtick block so linters can
parse them. Locate unlabeled fences near headings like "Ontology" and elsewhere,
pick the most specific language for the content in each block (use text for
plain examples), and update all ``` blocks consistently to ```text, ```json,
```elixir, ```bash, etc., ensuring no unlabeled fences remain.
Comment on lines
+73
to
+101
| pub fn parse_triples(ndjson: &str) -> Vec<OntologyTriple> { | ||
| ndjson | ||
| .lines() | ||
| .filter(|l| !l.trim().is_empty()) | ||
| .filter_map(|l| serde_json::from_str::<OntologyTriple>(l).ok()) | ||
| .collect() | ||
| } | ||
|
|
||
| /// Load an ndjson ontology document into a fresh [`SpoStore`]. | ||
| /// | ||
| /// Each triple's S/P/O labels are hashed to fingerprints via [`label_fp`] | ||
| /// (identity-by-name, deterministic), the edge is built with its NARS truth | ||
| /// value, and inserted under a content-addressed key `dn_hash("s|p|o")`. | ||
| /// | ||
| /// Returns the populated store; the triple count equals | ||
| /// `parse_triples(ndjson).len()` minus any exact `(s,p,o)` key collisions | ||
| /// (the extractor de-duplicates, so collisions are not expected). | ||
| pub fn load_ontology(ndjson: &str) -> SpoStore { | ||
| let mut store = SpoStore::new(); | ||
| for t in parse_triples(ndjson) { | ||
| let subj = label_fp(&t.s); | ||
| let pred = label_fp(&t.p); | ||
| let obj = label_fp(&t.o); | ||
| let record = SpoBuilder::build_edge(&subj, &pred, &obj, TruthValue::new(t.f, t.c)); | ||
| let key = dn_hash(&format!("{}|{}|{}", t.s, t.p, t.o)); | ||
| store.insert(key, &record); | ||
| } | ||
| store | ||
| } |
There was a problem hiding this comment.
Fail fast instead of silently dropping malformed ontology lines.
At Line 77, .ok() discards parse failures, so corrupted NDJSON can produce a partial SpoStore with no signal. Please return a Result from parse_triples/load_ontology and propagate line-level parse errors.
Suggested direction
+use snafu::{ResultExt, Snafu};
+
+#[derive(Debug, Snafu)]
+pub enum OntologyLoadError {
+ #[snafu(display("Invalid ontology NDJSON at line {line}: {source}"))]
+ ParseLine { line: usize, source: serde_json::Error },
+}
-pub fn parse_triples(ndjson: &str) -> Vec<OntologyTriple> {
+pub fn parse_triples(ndjson: &str) -> Result<Vec<OntologyTriple>, OntologyLoadError> {
ndjson
.lines()
- .filter(|l| !l.trim().is_empty())
- .filter_map(|l| serde_json::from_str::<OntologyTriple>(l).ok())
+ .enumerate()
+ .filter(|(_, l)| !l.trim().is_empty())
+ .map(|(line, l)| {
+ serde_json::from_str::<OntologyTriple>(l)
+ .context(ParseLineSnafu { line: line + 1 })
+ })
.collect()
}
-pub fn load_ontology(ndjson: &str) -> SpoStore {
+pub fn load_ontology(ndjson: &str) -> Result<SpoStore, OntologyLoadError> {
let mut store = SpoStore::new();
- for t in parse_triples(ndjson) {
+ for t in parse_triples(ndjson)? {
// ...
}
- store
+ Ok(store)
}As per coding guidelines crates/**/*.rs: “...reuse snafu error patterns”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/lance-graph/src/graph/spo/odoo_ontology.rs` around lines 73 - 101,
Change parse_triples to return Result<Vec<OntologyTriple>, Error> and make
load_ontology return Result<SpoStore, Error>, surfacing JSON parse errors
instead of silently discarding them; specifically replace the .filter_map(...
.ok()) in parse_triples with explicit per-line serde_json::from_str error
handling that yields a descriptive snafu error (include the offending line and
serde error) and propagate that Result up to load_ontology so it fails fast;
keep using label_fp, SpoBuilder::build_edge, dn_hash, TruthValue::new, and
SpoStore::insert but call them only after successful parsing, and add
appropriate snafu error variants and context per the crate's error patterns to
identify malformed NDJSON lines and I/O/format issues.
AdaWorldAPI
pushed a commit
that referenced
this pull request
May 29, 2026
Three correctness bugs + one architectural disclaimer + three polish items from the PR review pass. No new dependencies; all tests green. ## Bugs fixed ### 1. roundtrip_eq truth check inverted from default semantics (codegen_spine.rs:161 — `if tol > 0.0` gated the entire check) The doc claimed `truth_tolerance() = 0.0` meant "lossless / strictest." The code SKIPPED the truth check when tol = 0.0, so a projection that zeroed every (f, c) round-tripped green with no truth validation. The default-case bug was masked because the existing LossyDropFrequency test overrides tolerance to 0.01. Fix: drop the `if tol > 0.0` gate; the `abs() > tol` comparison naturally treats 0.0 as "any difference fails." Doc reworded to clarify "0.0 requires exact match; override to allow quantization." New test `default_tolerance_requires_exact_truth_match` pins the fix: a `TruthMangler` projection (preserves s/p/o, zeros f/c) MUST fail with the default tolerance. ### 2. parse_triples silently dropped malformed lines (odoo_ontology.rs:73-83 — `filter_map(|l| serde_json::from_str(l).ok())`) Lines that fail JSON parse were silently dropped. The `parses_all_triples` count assertion would catch *some* drift but a corrupted line that happens to keep counts aligned would go silent. Fix: new test `every_nonempty_line_parses` asserts the raw non-empty line count equals `parse_triples(ndjson).len()` — any silent drop fires the assertion. (Kept `parse_triples` returning `Vec` for API stability; the test catches what an error-returning API would.) ### 3. load_ontology collapsed duplicate (s,p,o) keys silently (odoo_ontology.rs:90-104 — HashMap last-write-wins via dn_hash) The module doc said "the extractor de-duplicates" — an unverified assumption. If the harvester ever emits two triples with the same (s, p, o) but different truth values, the second silently overwrites the first. Fix: new test `duplicate_spo_keys_are_last_write_wins` pins the overwrite semantics. A future switch to insertion-rejection or merge becomes a test failure instead of a silent behaviour change. ## Architectural disclaimer `OdooMethodKind` (codegen_spine.rs:230) is the bucket *catalogue*; the *classifier* (Rust port of `.claude/odoo/openings_hops.py`) is a follow-up emitter. action_emitter intentionally does NOT carry a `kind` field — the doc now spells out the wiring gap explicitly so the next session doesn't read this PR as "kind is wired" when it isn't. ## Polish - `RouteBucket::id_owned(&self) -> String` default method added (codegen_spine.rs:336-340). Escape hatch for async/iterator pipelines that need an owned id outside the borrow scope. - `impl std::error::Error for WidgetRenderError` (codegen_spine.rs) for ecosystem `?`-propagation compatibility. - `#[serde(deny_unknown_fields)]` on `OntologyTriple` so harvester schema drift fails parsing instead of silently dropping fields. - New test `function_count_matches_module_doc` in odoo_ontology.rs pins the "3 328 Functions" doc claim against drift inside the loader crate (was only asserted by the downstream action_emitter). - `compose_spec` now builds `effects` from the borrowed `effects_set` directly (action_emitter.rs:255-258), eliminating one redundant `idx.emits_by_fn.get(fn_id)` lookup. ## Test counts (orchestrator-verified) - lance-graph-contract codegen_spine: 6 → 7 passed (+1 truth-mangler) - lance-graph graph::spo:: total: 75 → 78 passed (+3 ontology tests) Fixes from PR #432 opus 4.8 reviewer (findings 1-3 must-fix, 5+7 should-discuss accepted, 4 doc-disclaimer accepted, 6+8 deferred, nitpicks 1+2+3 accepted).
…ama→GUI spine
Lands the codegen_spine module in lance-graph-contract per the
hardening direction: "triplets <> static codegen <> askama route SoC <>
askama gui shape — if you harden the codegen you have 32000 less of
token churn and technical debt because the links and SoC route
bucketing are already canonical."
Four contracts:
1. TripletProjection + roundtrip_eq — projections from triples to
const forms must round-trip equal under BTreeSet identity, with
optional NARS truth-tolerance on (f,c). Loss = build failure.
2. OdooMethodKind (16 stable-id variants, ALL[16] array, from_id
reverse lookup) + RouteBucket trait — one canonical bucket enum
across static codegen, askama route SoC, and GUI widget templates.
3. WidgetRender<B: RouteBucket> — askama widgets take typed buckets
only, never raw triples. Re-classification visible at type surface.
4. Genericity { Agnostic, Domain } — explicit marker for what to
codegen (Domain const) vs read from triple store at runtime (Agnostic).
Zero new dependencies (std-only: BTreeMap, BTreeSet, fmt, type_name).
Preserves the lance-graph-contract ZERO-DEPS invariant.
Tests (6/6): lossless_projection_passes_roundtrip,
lossy_projection_fails_roundtrip, odoo_method_kind_ids_are_unique_and_stable,
route_bucket_trait_compiles_with_concrete_impl,
widget_render_trait_compiles_with_dummy_impl,
genericity_marker_distinguishes_codegen_targets.
Cross-refs: I-VSA-IDENTITIES (bucket-by-enum at catalogue layer, not
similarity), E-THREE-PLANES-1 (runtime never crosses these boundaries),
E-FOUNDRY-LAYER-1 (typed domain layer this spine routes between).
Lands the deterministic projection of the Odoo business-logic graph
into the existing SpoStore. Extracted from odoo/addons/ by
ruff_python_dto_check AST analysis; ndjson serialised as
{s,p,o,f,c} per line; loaded via label_fp + SpoBuilder::build_edge +
TruthValue::new + store.insert(dn_hash(id), &rec).
Triple schema:
- (odoo:<family>, rdf:type, ogit:ObjectType) 388 structural
- (odoo:<fam>.<field>, rdf:type, ogit:Property) 3 107 structural
- (odoo:<fam>.<fn>, rdf:type, ogit:Function) 3 328 structural
- (odoo:<fam>.<field>, emitted_by, odoo:<fam>.<fn>) 3 228 body-write authoritative
- (odoo:<fam>.<field>, depends_on, odoo:<fam>.<dep>) 6 309 @api.depends authoritative
- (odoo:<fam>.<fn>, raises, exc:<Type>) ~451 body raise
- (odoo:<fam>.<fn>, reads_field, …) body-inferred
- (odoo:<fam>.<fn>, traverses_relation, …) body for-loop
NARS truth values carry provenance:
- structural edges (1.0, 1.0)
- decorator/body-authoritative edges (0.95, 0.90)
- body-inferred edges (0.85, 0.75)
The "a+b emits c through d?" Foundry query becomes a deterministic
graph deduction:
{c : c depends_on a ∧ c depends_on b} then {d : c emitted_by d}
NOT a similarity search.
Tests (4/4): parses_all_triples (22245), predicate_histogram_matches_extraction
(depends_on 6309, emitted_by 3228, rdf:type 6823), loads_into_spo_store_and_queries_forward,
emitted_by_edge_is_present (account_move.amount_total ← _compute_amount).
Also ships:
- `.claude/knowledge/foundry-workshop-elixir-rust-evaluation.md`: distillation
of how Foundry Workshop's low-code maps onto the Elixir-surface +
Rust-compile architecture (4 holds + 4 gaps).
- `.claude/knowledge/semantic-operational-handbook-v0.1.md`: primitive
catalogue underlying the four deliverables.
Data file odoo_ontology.spo.ndjson (2.5 MB) is regenerable via the
extraction pipeline; embedded only into tests via include_str! so the
release binary doesn't carry it.
Two deterministic string-layer emitters that consume parsed
OntologyTriple records and project them into Foundry-shape specs.
Live at the codegen layer above the fingerprint SpoStore so they
remain independent of internal hashing — the SpoStore stays the
runtime query path, these are build-time projections.
## action_emitter
`emit_actions(&[OntologyTriple]) -> Vec<ActionSpec>` composes one
record per function subject:
ActionSpec {
id, family,
effects ← reverse emitted_by,
inputs ← forward depends_on on each effect,
raises ← forward raises (guard signal),
reads ← forward reads_field (body-inferred),
traverses ← forward traverses_relation,
}
Classifiers: is_pure_guard() (raises, no effects → Foundry validation),
is_pure_compute() (effects, no raises → Foundry derive), is_trivial()
(no edges at all → harvester stub).
`emit_non_trivial_actions` filters trivials out. BTreeSet→sorted-Vec
collection guarantees byte-identical output across runs at the
(function, field) identity level (truth values intentionally dropped
at this coarser projection).
Tests (9/9): compose_account_move_compute_amount, pure_guard_classification,
output_is_sorted_deterministic, emit_non_trivial_drops_empties,
shipped_ontology_produces_expected_function_count (3328),
shipped_ontology_compute_amount_has_real_dependencies,
family_of_handles_dotted_iri, empty_triples_produce_empty_specs,
function_with_no_emits_has_empty_inputs.
## link_chain
`split_chain(&str) -> Option<LinkChain>` decomposes flat dotted
dependency IRIs into structured per-hop records:
"odoo:account_move.line_ids.matched_debit_ids.debit_move_id.move_id.line_ids.amount_residual"
─→ LinkChain {
source_family: "account_move",
hops: ["line_ids","matched_debit_ids","debit_move_id","move_id","line_ids"],
leaf: "amount_residual",
}
Plus `split_all_depends_on` (BTreeMap<subject, Vec<LinkChain>> with
sort+dedup) and `compute_stats` (SplitStats { depends_on_total,
well_formed, malformed, max_depth, direct_refs }).
Intentionally string-only. Target ObjectType resolution
(`account_move.line_ids → account.move.line`) needs the
OdooEntity::fields target table in `lance-graph-ontology`; adding
that dep here would create a backward edge in the crate graph.
Resolution is the consumer crate's job.
Robust: empty strings, empty segments (a..b, .a, a.), family-only
paths all return None so schema violations surface at the caller.
Tests (10/10): split_direct_field_reference, split_single_hop,
split_five_hop_real_path, split_handles_missing_prefix,
split_rejects_malformed (8 cases), split_all_depends_on_indexes_by_subject,
split_all_depends_on_dedups, shipped_ontology_decomposes_cleanly
(6309 depends_on, 0 malformed, max_depth ≥ 3),
shipped_ontology_amount_total_chains_present,
compute_stats_counts_each_category.
## Review pattern
Each module went through build (with `/// work` markers) →
opus-4.8 reviewer pass → marker removal → orchestrator-run cargo
verify. Reviewer-1 eliminated 4 BTreeSet::cloned() allocations via
a collect_sorted helper + 2 edge-case tests. Reviewer-2 collapsed
two-pass validation to a single split('.') loop, replaced
Vec::remove(0) with split_first/split_last, added 4 malformed-input
assertions + compute_stats coverage test.
…tology Pure rustfmt output (cargo fmt -p lance-graph). Line-wrapping preferences (collapse 4-line .get/.unwrap_or chains, expand single-line function signatures past 100 cols). No semantic changes. Fixes the `format` CI check on PR #432.
Three correctness bugs + one architectural disclaimer + three polish items from the PR review pass. No new dependencies; all tests green. ## Bugs fixed ### 1. roundtrip_eq truth check inverted from default semantics (codegen_spine.rs:161 — `if tol > 0.0` gated the entire check) The doc claimed `truth_tolerance() = 0.0` meant "lossless / strictest." The code SKIPPED the truth check when tol = 0.0, so a projection that zeroed every (f, c) round-tripped green with no truth validation. The default-case bug was masked because the existing LossyDropFrequency test overrides tolerance to 0.01. Fix: drop the `if tol > 0.0` gate; the `abs() > tol` comparison naturally treats 0.0 as "any difference fails." Doc reworded to clarify "0.0 requires exact match; override to allow quantization." New test `default_tolerance_requires_exact_truth_match` pins the fix: a `TruthMangler` projection (preserves s/p/o, zeros f/c) MUST fail with the default tolerance. ### 2. parse_triples silently dropped malformed lines (odoo_ontology.rs:73-83 — `filter_map(|l| serde_json::from_str(l).ok())`) Lines that fail JSON parse were silently dropped. The `parses_all_triples` count assertion would catch *some* drift but a corrupted line that happens to keep counts aligned would go silent. Fix: new test `every_nonempty_line_parses` asserts the raw non-empty line count equals `parse_triples(ndjson).len()` — any silent drop fires the assertion. (Kept `parse_triples` returning `Vec` for API stability; the test catches what an error-returning API would.) ### 3. load_ontology collapsed duplicate (s,p,o) keys silently (odoo_ontology.rs:90-104 — HashMap last-write-wins via dn_hash) The module doc said "the extractor de-duplicates" — an unverified assumption. If the harvester ever emits two triples with the same (s, p, o) but different truth values, the second silently overwrites the first. Fix: new test `duplicate_spo_keys_are_last_write_wins` pins the overwrite semantics. A future switch to insertion-rejection or merge becomes a test failure instead of a silent behaviour change. ## Architectural disclaimer `OdooMethodKind` (codegen_spine.rs:230) is the bucket *catalogue*; the *classifier* (Rust port of `.claude/odoo/openings_hops.py`) is a follow-up emitter. action_emitter intentionally does NOT carry a `kind` field — the doc now spells out the wiring gap explicitly so the next session doesn't read this PR as "kind is wired" when it isn't. ## Polish - `RouteBucket::id_owned(&self) -> String` default method added (codegen_spine.rs:336-340). Escape hatch for async/iterator pipelines that need an owned id outside the borrow scope. - `impl std::error::Error for WidgetRenderError` (codegen_spine.rs) for ecosystem `?`-propagation compatibility. - `#[serde(deny_unknown_fields)]` on `OntologyTriple` so harvester schema drift fails parsing instead of silently dropping fields. - New test `function_count_matches_module_doc` in odoo_ontology.rs pins the "3 328 Functions" doc claim against drift inside the loader crate (was only asserted by the downstream action_emitter). - `compose_spec` now builds `effects` from the borrowed `effects_set` directly (action_emitter.rs:255-258), eliminating one redundant `idx.emits_by_fn.get(fn_id)` lookup. ## Test counts (orchestrator-verified) - lance-graph-contract codegen_spine: 6 → 7 passed (+1 truth-mangler) - lance-graph graph::spo:: total: 75 → 78 passed (+3 ontology tests) Fixes from PR #432 opus 4.8 reviewer (findings 1-3 must-fix, 5+7 should-discuss accepted, 4 doc-disclaimer accepted, 6+8 deferred, nitpicks 1+2+3 accepted).
AdaWorldAPI
force-pushed
the
claude/activate-lance-graph-att-k2pHI
branch
from
f6f98e0 to
0e5ab98
Compare
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
2 participants
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.
Summary
Lands three Foundry-shape SPO emitters totalling ~1450 LOC + 25 tests, all green.
Closes deliverables 1 + 2 of the four-list in
.claude/knowledge/foundry-workshop-elixir-rust-evaluation.md.lance-graph-contract::codegen_spineFour canonical contracts gating the
triplets → static codegen → askama route SoC → askama GUI shapelayering:TripletProjection+roundtrip_eq— build-time gate that any const projection round-trips losslessly (set-eq on(s,p,o)+ truth tolerance).OdooMethodKind(16 stable-id variants,ALL[16],from_id) +RouteBuckettrait — one canonical bucket enum across every layer.WidgetRender<B: RouteBucket>— widgets take typed buckets, never raw triples.Genericity { Agnostic, Domain }— explicit "codegen const vs read at runtime" marker.Zero new dependencies (std-only). 6/6 tests.
lance-graph::graph::spo::odoo_ontologySPO loader for the Foundry-shape Odoo ontology — 22 245 triples (388 Object Types, 3 107 Properties, 3 328 Functions, plus
emitted_by/depends_on/raises/reads_field/traverses_relationedges with NARS truth values). 4/4 tests.lance-graph::graph::spo::action_emitterComposes per-function SPO edges into Foundry-shape
ActionSpecrecords:Plus
is_pure_guard()/is_pure_compute()/is_trivial()classifiers.Shipped ontology test: 22245 triples → 3328 functions,
account_move._compute_amountemitsamount_totalwith non-empty dependency closure. 9/9 tests.lance-graph::graph::spo::link_chainDecomposes flat dotted
depends_onpaths into per-hop chains. The 5-hop example from the evaluation doc verified end-to-end (odoo:account_move.line_ids.matched_debit_ids.debit_move_id.move_id.line_ids.amount_residual). Intentionally string-only — target ObjectType resolution stays in the consumer crate to keep the lance-graph crate graph acyclic. 10/10 tests.Architecture invariants preserved
lance-graph-contractremains zero-dep std-only.lance-graphemitters live at the string/codegen layer above the fingerprintSpoStore; no internal API touched.Review pattern (per session directive)
Each new module went through:
build (with
/// workmarkers) → opus-4.8 reviewer pass (idiomatic Rust, test coverage, marker removal) → orchestrator-run cargo verify.Reviewer-1 (action_emitter): eliminated 4
BTreeSet::cloned()allocations via a borrow-instead-of-allocate helper + 2 new edge-case tests.Reviewer-2 (link_chain): collapsed two-pass validation into a single
split('.')loop, replaced O(n)Vec::remove(0)with O(1)split_first/split_last; added 4 malformed-input assertions + 1 test pinning everySplitStatsfield.Test plan
cargo test -p lance-graph-contract --lib codegen_spine→ 6/6cargo test -p lance-graph --lib graph::spo::odoo_ontology→ 4/4cargo test -p lance-graph --lib graph::spo::action_emitter→ 9/9cargo test -p lance-graph --lib graph::spo::link_chain→ 10/10cargo test -p lance-graph --lib graph::spo::(all SPO) → 75/75Out of scope (deliverables 3+4, next PR cycle)
ir.model.access.csv→ role-permission triples (closes Gap 3 in the evaluation doc).Generated by Claude Code
Summary by CodeRabbit
Documentation
New Features
Chores
Tests