Skip to content

chore(typedb): migrate typedb-driver 3.8 → 3.11.5#98

Merged
rsthornton merged 2 commits into
mainfrom
chore/typedb-driver-3.11.5
Jun 15, 2026
Merged

chore(typedb): migrate typedb-driver 3.8 → 3.11.5#98
rsthornton merged 2 commits into
mainfrom
chore/typedb-driver-3.11.5

Conversation

@rsthornton

Copy link
Copy Markdown
Contributor

Unblocks the workspace build / release path (#96). The 3.8 → 3.11.5 driver bump is an API break; this migrates both call sites.

Changes

  • DriverOptions::new(bool, Option<_>)DriverOptions::new(DriverTlsConfig::disabled()) (no longer fallible).
  • TypeDBDriver::new(&str, …) → takes Addresses via Addresses::try_from_address_str(host).
  • Both call sites migrated: src-tauri/src/typedb_reader.rs::connect() and tools/bert-typedb/src/driver.rs::Transpiler::connect().
  • insert.rs: adds the ProcessPrimitive::Amplifying match arm so the tool stays exhaustive against the current bert-core enum (the "Amplifying" string already exists in schema.tql — three-way consistent).

Why this was parked

It was finished-but-uncommitted local WIP, set aside during bert#88 so the kernel diff stayed clean (the TypeDB path isn't on the critical path — sims run without it). Now landed on its own so it's reviewed/CI'd independently.

Verification

  • cargo check --workspace clean (previously red on main — this is the fix).
  • bert-typedb default suite passes.
  • Integration smoke test passes against a live TypeDB 3.11.5: cargo test -p bert-typedb --features integration --test driver_smokeconnect_and_roundtrip_schema (connect → create db → roundtrip schema → drop) + schema_constant_is_nonempty, 2/2.

Closes #96

🤖 Generated with Claude Code

The 3.11 driver changed two call shapes:
- DriverOptions::new(bool, Option<_>) → DriverOptions::new(DriverTlsConfig)
  and is no longer fallible.
- TypeDBDriver::new(&str, ..) now takes Addresses, built via
  Addresses::try_from_address_str.

Migrated both call sites: src-tauri's connect() and the bert-typedb
Transpiler. Also adds the ProcessPrimitive::Amplifying match arm in
insert.rs so the tool stays exhaustive against the current bert-core
enum (the "Amplifying" string already exists in schema.tql).

Verified: cargo check --workspace clean; the bert-typedb integration
smoke test (connect + schema roundtrip) passes against a live
TypeDB 3.11.5 server.

Closes #96

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown

Code Review: chore(typedb): migrate typedb-driver 3.8 → 3.11.5

Summary: Clean, focused migration that unblocks the workspace build. The changes are minimal and correct — this is a nice example of a well-scoped chore PR.


What the PR does

  • Bumps typedb-driver from 3.8 → 3.11.5 in both src-tauri/Cargo.toml and tools/bert-typedb/Cargo.toml
  • Migrates two call sites to the new API:
    • DriverOptions::new(false, None) (fallible) → DriverOptions::new(DriverTlsConfig::disabled()) (infallible)
    • TypeDBDriver::new(&str, …)TypeDBDriver::new(Addresses, …) via Addresses::try_from_address_str
  • Adds the missing ProcessPrimitive::Amplifying arm in insert.rs to keep the match exhaustive

Strengths

  • Error handling improved in driver.rs: Addresses parse errors now map to the structured TranspilerError::Connection variant with the host field populated — significantly better diagnostics than the old TranspilerError::Internal("DriverOptions construction failed: …") path.
  • Infallible DriverOptions::new(): Removing the now-unnecessary .map_err() on options construction is the right call.
  • ProcessPrimitive::Amplifying placement: Matches the existing enum declaration order and the "Amplifying" string already present in schema.tql — three-way consistency is good to call out explicitly.
  • Exact version pin (3.11.5): Appropriate for an API-breaking dependency; avoids silent future churn.

Minor Issues

1. Inconsistent error paths for Addresses parse failure between the two call sites

In typedb_reader.rs (line 14), an Addresses parse error is flattened to a plain string:

Addresses::try_from_address_str(DEFAULT_HOST).map_err(|e| format!("Addresses: {e}"))?;

In driver.rs (lines 62-65), the same error routes through the structured TranspilerError::Connection variant. This is fine since typedb_reader.rs::connect() already returns Result<TypeDBDriver, String> (pre-existing design), but it's worth noting the asymmetry if that module ever gets a proper error type.

2. Hardcoded credentials in typedb_reader.rs (pre-existing, not introduced here)

const DEFAULT_PASSWORD: &str = "password";

Not a regression from this PR, but since you're touching this file it might be worth a // TODO: read from config note or linking it to a follow-up issue if this module is ever exposed beyond local dev.

3. Consider whether "3.11" semver range is safer long-term

Pinning to "3.11.5" means missing future patch releases automatically. Given that this was an API break at the minor version, "~3.11" (patch-compatible) would be equally safe and pick up bug fixes. Optional — exact pin is defensible.


No issues found in

  • insert.rs change: correct, minimal, in declaration order
  • Both Cargo.toml bumps: consistent
  • Import groupings: clean, idiomatic

Checklist per CLAUDE.md quality gates

  • cargo fmt --all — not shown in PR, assumed run
  • cargo clippy --all-targets -- -D warnings — not shown, assumed clean given cargo check passes
  • Integration test: connect_and_roundtrip_schema + schema_constant_is_nonempty pass against live TypeDB 3.11.5 ✓

Verdict: Approve. The migration is correct, the error handling improvement in driver.rs is a genuine quality win, and the Amplifying fix keeps the codebase consistent with the schema. Minor notes above are non-blocking.

🤖 Generated with Claude Code

cargo fmt reflow of the Addresses/TypeDBDriver error-mapping added in
the previous commit — no behavior change. Closes the fmt quality gate
the migration commit missed.
@rsthornton rsthornton merged commit 0ad5183 into main Jun 15, 2026
1 check passed
@rsthornton rsthornton deleted the chore/typedb-driver-3.11.5 branch June 15, 2026 23:16
@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown

Code Review

Overall: Approve with minor suggestions. This is a clean, necessary migration that unblocks the build. Changes are minimal, correct, and consistent across both call sites.


What the PR does

Migrates typedb-driver from 3.8 → 3.11.5 across two call sites, adapting to two breaking API changes:

  1. DriverOptions::new(bool, Option<_>)DriverOptions::new(DriverTlsConfig::disabled()) (now infallible)
  2. TypeDBDriver::new(&str, …) → takes an Addresses value (host string is parsed separately via Addresses::try_from_address_str)

Also adds the missing ProcessPrimitive::Amplifying match arm in insert.rs to restore exhaustiveness.


Correctness ✅

  • Both call sites (typedb_reader.rs and driver.rs) are migrated identically — no divergence.
  • Removing .map_err on DriverOptions::new is correct: the new API is infallible, so the old ? was never needed.
  • Addresses::try_from_address_str is correctly treated as fallible and the error is surfaced.
  • The ProcessPrimitive::Amplifying => "Amplifying" addition is three-way consistent: the variant exists in bert-core/src/lib.rs:1158, the string "Amplifying" is listed in schema.tql:61, and the match arm now reflects it.

Minor concerns

driver.rs: Error variant ambiguity for address parse failure

Addresses::try_from_address_str(host).map_err(|e| TranspilerError::Connection {
    host: host.to_string(),
    source: Box::new(e),
})?;

A bad host string (e.g. "not-a-valid-host") and a refused TCP connection both surface as TranspilerError::Connection. These are distinct failure modes — the first is a programming error, the second is a runtime environment problem. A TranspilerError::InvalidAddress variant would make the distinction clear in error messages and test assertions. Low priority, but worth tracking if TranspilerError grows.

typedb_reader.rs: Hardcoded default credentials

const DEFAULT_USERNAME: &str = "admin";
const DEFAULT_PASSWORD: &str = "password";

These are pre-existing (not introduced by this PR) and the PR description notes they match TypeDB CE out-of-box defaults. Still worth flagging: if this path is ever exposed beyond localhost dev use, these should come from config or environment variables rather than constants.

ProcessPrimitive::Amplifying — no unit test coverage

The process_primitive_str function now covers all 10 variants, but Amplifying is not exercised in bitcoin_roundtrip.rs or any other test (the roundtrip test fixture appears not to use an Amplifying node). This isn't blocking — the compiler enforces exhaustiveness — but a one-liner test for process_primitive_str(ProcessPrimitive::Amplifying) == "Amplifying" would guard against future string-rename mistakes.


What's not a concern

  • Pinning "3.11.5" (Cargo treats this as ^3.11.5) is appropriate given the breaking minor-version churn in this driver.
  • The coarse Result<TypeDBDriver, String> error type in typedb_reader.rs is pre-existing and outside this PR's scope.
  • The PR description claims cargo check --workspace was red on main before this — that's consistent with the missing Amplifying arm (non-exhaustive match = compile error in Rust).

Verdict

The migration is correct and the diff is tight. The two minor concerns above (TranspilerError disambiguation, test coverage for Amplifying) are follow-up candidates, not blockers.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bert-tauri/bert-typedb don't compile against typedb-driver 3.11.5 (workspace build + release path)

1 participant