Skip to content

feat: Python SDK (v0.1.0) — programmatic project access#93

Merged
marklubin merged 5 commits intomainfrom
mark/sdk-v1
Mar 9, 2026
Merged

feat: Python SDK (v0.1.0) — programmatic project access#93
marklubin merged 5 commits intomainfrom
mark/sdk-v1

Conversation

@marklubin
Copy link
Copy Markdown
Owner

@marklubin marklubin commented Mar 9, 2026

Closes #57

Summary

  • Python SDK (synix.sdk) — complete programmatic access: init, open, source management, build, release, search, inspect
  • Entry point: synix.open_project(path) / synix.init(path, pipeline=...)
  • Core classes: Project, Release, SearchHandle, SdkSource
  • Fail-closed embedding enforcement: missing embeddings with declared embedding_config raises EmbeddingRequiredError
  • Frozen dataclasses: SdkArtifact, SdkSearchResult, BuildResult
  • Error hierarchy: SdkErrorSynixNotFoundError, ReleaseNotFoundError, ArtifactNotFoundError, SearchNotAvailableError, EmbeddingRequiredError, ProjectionNotFoundError, PipelineRequiredError

Review Fixes

Round 1

Finding Fix
open() shadows Python builtin (Claude) Renamed to open_project()
Fixed scratch dir concurrent stomping (GPT critical) UUID-based scratch_{uuid} dirs under .synix/work/
build() mutates pipeline in-place (both) Deep-copy pipeline before mutation
source() fallback creates undeclared sources (GPT) Removed fallback, raises SdkError with declared source list
flat_file() raises SearchNotAvailableError (GPT) Added ProjectionNotFoundError
flat_file()/flat_file_path() duplication (Claude) Extracted _resolve_flat_file_path()
_get_closure KeyError on malformed receipt (Claude) Wrapped in ReleaseNotFoundError
_from_snapshot_dict dead code (Claude) Removed

Round 2

Finding Fix
open still re-exported from __init__.py (both) Removed — no longer shadows builtin at package level
SdkError not exported (GPT) Added to __init__.py exports
Path traversal in SdkSource.add_text/remove (GPT critical) _validate_name() rejects ../, absolute paths, nested paths
Scratch release race: closure re-resolves HEAD independently (Claude) _get_closure() reads snapshot_oid from receipt written by execute_release
sdk-design.md uses deprecated synix.open() (Claude) Updated all examples to open_project
Scratch close() leaks receipt dir (implicit) Cleans up both work/ scratch dir and releases/ receipt dir

Test Coverage

  • tests/e2e/test_sdk.py — 71 tests (init, open, source, build, release, search, inspect, errors, path traversal)
  • tests/e2e/test_sdk_incremental.py — 9 tests (full lifecycle: build → add → rebuild → release → search)
  • tests/e2e/test_release_embeddings.py — 6 tests (embedding generation, fail-closed enforcement)

Introduces synix.open(path) and synix.init(path, pipeline=...) entry
points with Project, Release, SearchHandle, and typed error hierarchy.
Supports build, release, search, artifact inspection, and ref listing
through a stable Python API without touching CLI or internals directly.

68 e2e tests + 18 incremental cache tests covering build idempotency,
release lifecycle, search correctness, and content-addressed dedup.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026

Note

Red Team Review — OpenAI GPT-5.4 | Adversarial review (docs + diff only)

Threat assessment — Medium-high risk: this PR adds a new public SDK surface on top of unstable internals, and several API/behavior choices look sticky, under-validated, and not aligned with the project’s own stated boundaries.

One-way doors

  1. Top-level public SDK in synix.__init__ — Exporting open, init, Project, Release, result types, and SDK_VERSION from src/synix/__init__.py makes this a first-class public API immediately. Hard to reverse because users will import from synix directly. Safe only if API stability expectations, naming, and compatibility policy are explicitly agreed.
  2. open() / init() namingsynix.open() shadows Python mental models and becomes core docs/examples API. Renaming later is painful. Safe only if the team is sure this is the long-term ergonomic entrypoint.
  3. SDK version independent from package versiondocs/sdk.md says SDK semver is independent from package version. That’s a contract. Hard to maintain if the SDK ships in the same package. Safe only if release/versioning mechanics actually support dual version semantics.
  4. release("HEAD") scratch semantics — This creates a behavior users will rely on. Current implementation uses a fixed work path and side effects. Safe only if scratch realization is concurrency-safe and formally specified.

Findings

  1. src/synix/sdk.py — mutating pipeline objects in Project.build()
    It rewrites resolved.llm_config, resolved.synix_dir, resolved.source_dir, resolved.build_dir, and Source.dir in place. That leaks execution concerns into the declarative pipeline model and can corrupt caller-held pipeline objects across builds/tests. Violates Python-first declaration cleanliness and invites surprising cache/path behavior. Severity: [warning]

  2. src/synix/sdk.py — fixed scratch directory .synix/work/scratch_release
    Release._release_dir() uses a single hardcoded scratch path for all release("HEAD") calls. Two processes/threads will stomp each other; even sequential nested usage is unsafe. This is a reliability bug, not just an optimization issue. Severity: [critical]

  3. src/synix/sdk.py — docs promise “lazy everything”, code eagerly materializes releases/search state
    release("HEAD") triggers materialization on first access; search() instantiates CLI search plumbing directly. The design doc claims ergonomic wrapper over primitives, but this couples SDK to CLI module synix.cli.search_commands.ReleaseProvenanceProvider. That is backwards layering. Severity: [warning]

  4. src/synix/sdk.py — source management can target undeclared sources
    Project.source() falls back to pipeline.source_dir / name if no Source layer matches. That silently creates writable undeclared inputs, weakening DAG integrity and provenance assumptions. Boundary validation is missing. Severity: [warning]

  5. src/synix/sdk.py / docs/sdk.md — API shape diverges from existing product language
    README/design center CLI build/release separation and releases as named materializations. SDK adds direct filesystem-like source mutation, flat_file() accessors, and build() timeout/concurrency knobs with no equivalent in public docs. This expands the mental model substantially without reconciling it with the design docs. Severity: [warning]

  6. src/synix/sdk.py — incorrect/fragile error taxonomy reuse
    flat_file() raises SearchNotAvailableError when the projection is missing. That is semantically wrong and bakes bad exception contracts into public API. Similar issue for generic FileNotFoundError leaking internal storage expectations. Severity: [minor]

  7. src/synix/sdk.py — release search assumes single search.db at release root
    Multiple search surfaces are supported conceptually, but search() always opens release_dir / "search.db". Surface selection only filters declarations, not storage location. This looks like hidden coupling to current adapter layout and will break if per-surface outputs diverge. Severity: [warning]

  8. docs/sdk.md — documents methods not exported/implemented consistently
    Docs mention Transform import in quick start and project.source(...).remove, clear, project.clean(), and context-manager semantics. Some exist, some aren’t exported from __init__, and docs/sdk-design.md public API omits methods later documented. Public docs are already inconsistent at birth. Severity: [minor]

Missing

  • Unit tests for concurrent release("HEAD") access and multi-process safety.
  • Tests for init()/build() when called on an existing project, partial .synix, invalid pipeline, invalid source names, path traversal labels, missing input files, or release failures.
  • Tests proving SDK doesn’t mutate caller pipeline definitions unexpectedly.
  • Documentation updates in README / website for this new major public API. CLAUDE.md and new docs are not enough for a user-facing surface.
  • Any stated compatibility policy for this SDK in a pre-1.0 project.

VerdictBlock: the SDK is a major new public API, but it currently hard-codes unsafe scratch behavior, leaks internals, and ships with unresolved contract/design inconsistencies.

Review parameters
  • Model: gpt-5.4
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 2,666 lines
  • Prompt: .github/prompts/openai_review.md
  • Timestamp: 2026-03-09T06:16:51Z

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026

Note

Architectural Review — Claude Opus | Blind review (docs + diff only)

Summary

This PR adds a Python SDK (synix.sdk) providing programmatic access to Synix — project init, source management, build, release, search, and artifact inspection. It includes the implementation module, a design doc, a user-facing API guide, and ~1250 lines of e2e tests covering both unit-level SDK operations and a full incremental pipeline lifecycle.

Alignment

Strong fit. DESIGN.md §4.1 explicitly bets on Python-first over config, and this SDK is the natural extension: pipelines defined in Python can now be driven from Python. The data plane reads go through releases (consistent with build/release separation in §1.5), artifacts are immutable and content-addressed, and the incremental rebuild tests validate cache key semantics from §3.3. The "architecture is a runtime concern" hypothesis is served directly — an agent runtime can now swap pipelines programmatically. The v2 buffer sketch in sdk-design.md maps cleanly to the Letta model without committing prematurely.

Observations

  1. [positive] test_sdk_incremental.py is excellent — tests the full lifecycle (build → add source → incremental rebuild → release → search), validates cache hits, content-addressed identity across rebuilds, source removal, and ref tracking. This is exactly the coverage needed for the core value prop.

  2. [positive] Fail-closed embedding enforcement in Release.search() is well-designed. The three-way check (declared modes vs requested mode vs actual embeddings on disk) prevents silent degradation, with clear error types.

  3. [concern] synix.open shadows the Python builtin open. The re-export in __init__.py means from synix import open will shadow it in any importing module. This will bite users. Consider synix.open_project or relying solely on synix.open() as a module-qualified call without re-exporting the name.

  4. [concern] Release._get_closure() imports ReleaseClosure from synix.build.release, but the scratch path uses ReleaseClosure.from_ref() while the named path uses ReleaseClosure.from_snapshot(). If receipt.json is malformed or missing snapshot_oid, this will throw a KeyError rather than an SdkError. The error hierarchy promises clean exceptions but this path isn't wrapped.

  5. [question] Project.build() mutates the pipeline object in-place — resolving relative paths to absolute, setting synix_dir, overriding timeout in llm_config. If a user holds a reference to the pipeline and calls build() twice from different Project instances (e.g., in a benchmark harness like the LENS use case), the second call operates on an already-mutated pipeline. Should build() work on a copy?

  6. [concern] flat_file() and flat_file_path() have near-identical bodies (~15 lines each). Extract the lookup logic into a shared _resolve_flat_file(name) -> Path method.

  7. [nit] SdkArtifact._from_snapshot_dict is defined but never called in this diff. Dead code or forward-looking? If forward-looking, a comment would help.

  8. [question] SearchHandle delegates to Release.search() with surface=self._projection_name, but _resolve_search_projection does the lookup every time. For repeated queries (the stated use case for SearchHandle), caching the resolved projection would avoid redundant dict scans.

  9. [nit] SdkLayer exposes name, level, count as bare instance attributes (set in __init__) rather than as a dataclass. Inconsistent with the frozen dataclass pattern used for SdkArtifact, SdkSearchResult, and BuildResult. Makes SdkLayer mutable where sibling types aren't.

  10. [positive] The CLI↔SDK coverage table in sdk.md is a smart inclusion — makes the mapping explicit and reveals any gaps. Every CLI command has an SDK equivalent.

  11. [concern] Thread safety section says "not thread-safe" but the LENS benchmark use case implies concurrent evaluation runs. This is fine as documented, but Project.build() mutating shared pipeline state (observation 5) makes even single-threaded multi-project usage risky.

Verdict

This is a strong incremental step — it makes Synix embeddable as infrastructure (the "boto3" framing is apt), the test coverage is thorough, and the design respects build/release separation. The open shadowing and pipeline mutation issues should be fixed before merge.

Review parameters
  • Model: claude-opus-4-6
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 2,666 lines
  • Prompt: .github/prompts/claude_review.md
  • Timestamp: 2026-03-09T06:17:01Z

- Rename `open()` to `open_project()` to avoid shadowing Python builtin;
  keep `open` as deprecated alias for backwards compatibility
- Use UUID-based scratch dir for `release("HEAD")` — prevents concurrent
  stomping (GPT critical finding)
- Deep-copy pipeline in `build()` to prevent caller mutation (both reviews)
- Remove `source()` fallback to undeclared sources — now raises SdkError
  with list of declared sources (GPT warning)
- Add `ProjectionNotFoundError` — `flat_file()` no longer raises
  `SearchNotAvailableError` (GPT minor, wrong error taxonomy)
- Extract `_resolve_flat_file_path()` — dedup flat_file/flat_file_path
  (Claude concern)
- Wrap `_get_closure()` receipt parsing in SdkError (Claude concern)
- Remove dead `SdkArtifact._from_snapshot_dict` (Claude nit)
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026

Note

Red Team Review — OpenAI GPT-5.4 | Adversarial review (docs + diff only)

Threat assessment — Medium-high risk: this PR adds a new public SDK surface with several irreversible API decisions, while the docs and implementation already disagree in ways users will notice immediately.

One-way doors

  1. Top-level SDK API in src/synix/__init__.py (init, open_project, deprecated open, Project, Release) — This becomes part of the package’s public mental model and import surface. Reversing naming or object model later will be painful once examples depend on it. Safe only if the API names, lifecycle semantics, and error model are aligned across code, docs, and README.
  2. project.release("HEAD") as an ephemeral scratch realization — That bakes lifecycle semantics into the SDK. If later HEAD should mean “read current ref directly” rather than “materialize a temp release,” changing behavior will break callers and resource assumptions. Safe only if concurrency, cleanup, and failure semantics are specified and tested.
  3. Independent SDK_VERSION = "0.1.0" semver story in docs — This creates a second version contract separate from package version. Hard to walk back once external consumers key behavior off it. Safe only if release/versioning policy is documented project-wide and enforced in packaging/release tooling.
  4. SDK source-management abstraction (project.source(...).add/remove/clear) — This commits to filesystem mutation as the control-plane contract. If source backends become non-filesystem later, these semantics may not generalize. Safe only if the project is willing to support this abstraction long-term.

Findings

  1. docs/sdk-design.md vs docs/sdk.md vs src/synix/sdk.py naming drift — design says synix.open(), docs say open_project() preferred, code exports both and README/website mention neither. This is avoidable public API confusion on day one. Severity: [warning]

  2. src/synix/sdk.py: Project.build() deep-copies the pipeline and mutates path fields — You’re encoding runner/path quirks into the SDK instead of fixing the runner boundary. That leaks implementation details and risks divergence from CLI behavior when pipeline objects carry non-copyable state or custom callables. Severity: [warning]

  3. src/synix/sdk.py: Release.search() hardcodes release_dir / "search.db" — Search projections are supposed to be adapter-based and named surfaces, but implementation assumes a single canonical file layout. That is tight coupling to one adapter/materialization convention and contradicts the “extensible” SDK claim. Severity: [critical]

  4. src/synix/sdk.py: _load_embedding_provider() checks release_dir / "embeddings" / "manifest.json" without surface scoping — Multiple search surfaces with different embedding configs will collide on one shared embeddings path assumption. Hidden complexity is being hand-waved away in docs. Severity: [warning]

  5. src/synix/sdk.py: Release._release_dir() scratch creation under .synix/work/scratch_* — No locking, no cleanup on crash, no collision/race strategy beyond random suffix. Multi-process access to the same project can leave orphaned scratch dirs or inconsistent release materialization. Severity: [warning]

  6. src/synix/sdk.py: SdkSource.add() blindly copy2s by basename — Adding two files with the same filename from different paths silently overwrites. That’s a bad system boundary for imported source data and will corrupt user expectations. Severity: [warning]

  7. src/synix/sdk.py: SdkSource.add_text() and remove() accept arbitrary labels/names — No validation against path traversal (../x), absolute paths, or nested paths. This lets SDK callers write/delete outside the source dir. Severity: [critical]

  8. src/synix/sdk.py: Release.lineage() returns art.provenance_chain artifacts — Docs promise “lineage” semantics, but implementation just reflects precomputed BFS labels and may omit structure/order expectations. That’s weaker than the design’s provenance DAG story and underspecified for consumers. Severity: [minor]

  9. src/synix/__init__.py exports deprecated open anyway — You know it shadows builtin open, docs tell users not to use it, but you still promote it at package top level. That’s self-inflicted API debt. Severity: [minor]

  10. docs/sdk.md claims CLI↔SDK coverage including project.clean() and source removal, but __init__.py does not export error classes or SdkError hierarchy — User-facing docs emphasize structured SDK error handling, but top-level API doesn’t expose those exceptions consistently. Severity: [minor]

Missing

  • Unit tests for path validation and overwrite behavior in SdkSource.add/add_text/remove.
  • Tests for multi-surface releases with different search.db / embeddings layouts.
  • Tests for concurrent release("HEAD") access and scratch cleanup after exceptions/crashes.
  • Documentation updates in README and website content introducing the SDK; right now this is a significant user-facing feature added only to CLAUDE/docs.
  • Clear compatibility statement for SDK_VERSION vs package version.
  • Tests proving SDK build behavior matches CLI build behavior for path resolution and timeout overrides.

VerdictBlock: this is a public API addition with real one-way doors, and the current implementation has unsafe file-boundary handling plus hardcoded search/release assumptions that undercut the project’s stated extensibility.

Review parameters
  • Model: gpt-5.4
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 2,673 lines
  • Prompt: .github/prompts/openai_review.md
  • Timestamp: 2026-03-09T21:41:51Z

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026

Note

Architectural Review — Claude Opus | Blind review (docs + diff only)

Summary

Adds a Python SDK (synix.sdk) providing programmatic access to Synix — project init, source management, build, release, search, artifact inspection, and lineage. Includes a design doc, user-facing guide, ~760 lines of implementation, and ~1260 lines of e2e tests covering both unit-level SDK operations and incremental pipeline lifecycles.

Alignment

Strong fit. DESIGN.md §4.1 explicitly chose Python-first because "experimentation is a code activity." The SDK is the natural completion of that bet — pipelines defined in Python can now be executed in Python without shelling out. The build/release separation (§1.5) is preserved: project.build() produces snapshots, project.release_to() materializes projections. Artifacts remain immutable and content-addressed; the SdkArtifact dataclass is frozen and read-only. Provenance chains flow through release.lineage(). Cache semantics are untouched — the SDK delegates to the existing runner/planner. The Release read boundary (all reads go through a release) enforces the snapshot consistency model from the projection-release-v2 RFC.

Observations

  1. [concern] sdk.py:open = open_project shadows Python's builtin open. The deprecation note exists, but re-exporting it from __init__.py means from synix import open silently replaces the builtin in any module that does from synix import *. Consider removing the re-export now rather than shipping a known footgun at 0.1.

  2. [concern] sdk.py line ~370: _load_embedding_provider returns None when manifest.json is missing, but the caller then raises EmbeddingRequiredError. The error message says "Re-run release to generate embeddings" — but the actual failure could be a corrupted release directory. The error path is correct but the message is potentially misleading.

  3. [concern] Release._get_closure() imports ReleaseClosure from synix.build.release, but the scratch path calls ReleaseClosure.from_ref(self._synix_dir, "HEAD") after already materializing via execute_release. This means the closure resolves HEAD independently of the scratch materialization — if another build lands between materialization and closure resolution, you'd read a different snapshot than you materialized. Narrow race, but real in concurrent use.

  4. [question] docs/sdk.md documents synix.open() as deprecated but docs/sdk-design.md use-case examples all use synix.open(). Which is canonical? New users will copy the design doc examples.

  5. [question] Project.build() does copy.deepcopy(original) on the pipeline to avoid mutation. Is Pipeline (and all its layers, including LLM configs and transform closures) safely deep-copyable? Closures and bound methods can behave unexpectedly under deepcopy.

  6. [nit] SdkSource.remove() silently succeeds when the file doesn't exist. The test asserts removal works, but there's no test for removing a nonexistent file. Worth a one-liner test or an explicit design note.

  7. [nit] sdk.py uses both from synix.build.release import ReleaseClosure and from synix.build.release_engine import execute_release — two different modules with similar names (release vs release_engine). Not an SDK problem, but worth noting for legibility.

  8. [positive] Fail-closed embedding enforcement is well-designed. The three-way check (declared modes × requested mode × embedding presence) with specific error types is exactly right for an infrastructure API.

  9. [positive] test_sdk_incremental.py is excellent — it exercises the full lifecycle (init → add sources → build → release → search → add more → incremental rebuild → verify caching) with real FTS5 and real file I/O. The test_rebuild_no_changes_fully_cached test directly validates the content-addressed identity invariant.

  10. [positive] The v2 buffer concept in sdk-design.md is clearly scoped as deferred but designed-for. The Letta model mapping table is a useful framing device.

  11. [concern] Project.refs() hardcodes three prefix strings (refs/heads, refs/releases, refs/runs). If new ref namespaces are added, this silently omits them. Consider a ref_store.iter_all() or similar.

Verdict

This is a well-structured, well-tested incremental step that makes Synix embeddable as a library — a necessary precondition for the agent-runtime and benchmark use cases the project targets. Ship it after addressing the open builtin shadowing and the scratch-release race condition.

Review parameters
  • Model: claude-opus-4-6
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 2,673 lines
  • Prompt: .github/prompts/claude_review.md
  • Timestamp: 2026-03-09T21:42:07Z

…atch race

- Remove `open` re-export from __init__.py — no longer shadows builtin
  (both reviews, v2)
- Export `SdkError` from __init__.py for user-facing error handling
- Add path traversal validation in SdkSource — rejects `../` and
  absolute paths in add_text/remove (GPT critical)
- Fix scratch release race: _get_closure reads snapshot_oid from receipt
  written by execute_release, not by re-resolving HEAD independently
  (Claude concern #3)
- Scratch close() cleans up both work/ and releases/ dirs
- Update sdk-design.md examples to use open_project (Claude question #4)
- Update sdk.md quick start to use open_project
- All tests use open_project; 3 new tests for path traversal + undeclared source
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026

Note

Red Team Review — OpenAI GPT-5.4 | Adversarial review (docs + diff only)

Threat assessment — Medium-high risk: this PR ships a new public SDK surface from zero to “boto3 for synix” in one shot, with multiple irreversible API decisions and several mismatches against the documented model.

One-way doors

  1. Top-level public SDK API in src/synix/__init__.py
    Exporting Project, Release, init, open_project, dataclasses, and versioning makes this part of the stable user mental model immediately. Renaming or reshaping later will be painful. Safe only if the team is willing to support these names/semantics through v1 or explicitly mark the entire SDK experimental in docs and module naming.

  2. project.release("HEAD") as ephemeral scratch realization
    This is now a user-facing semantic contract. If HEAD later means “latest built snapshot” vs “materialize-on-read dev view”, you’ve locked in ambiguity. Safe only if this meaning is aligned with CLI/docs and tested for lifecycle/concurrency behavior.

  3. Independent SDK_VERSION semver policy in docs/sdk.md
    You’re declaring versioning policy users may automate against, separate from package version. That is hard to retract. Safe only if release tooling and compatibility policy actually support independent SDK versioning.

  4. Source management API (source.add/add_text/remove/clear)
    This defines filesystem abstraction semantics users will depend on. Reversing copy-vs-link behavior, filename restrictions, or source resolution logic later will break integrations. Safe only if these behaviors are intentional and documented as stable.

Findings

  1. src/synix/sdk.py + docs/sdk.md: API/docs mismatch on errors and methods
    Docs advertise synix.open() deprecated alias and importable errors like ReleaseNotFoundError, ArtifactNotFoundError, SearchNotAvailableError, but __init__.py exports only a subset and no open. Users following docs will hit import/runtime failures. Severity: [warning]

  2. src/synix/sdk.py: init() creates .synix but does not ensure project root exists
    ObjectStore(synix_dir) assumes parent path existence. synix.init("~/my-project") in docs may fail unless the directory already exists. That’s a bad system-boundary assumption. Severity: [warning]

  3. src/synix/sdk.py: Project.build() mutates path semantics around deprecated build_dir
    README/design say .synix/ is the single source of truth and “there is no build/ directory.” Yet SDK examples and tests still construct pipelines with build_dir="./build" and path-rewrite it. That directly contradicts the project’s stated build/release separation. Severity: [warning]

  4. src/synix/sdk.py: Release.search() hardcodes search.db at release root
    This leaks current adapter file layout into the SDK. If multiple search projections materialize different files or adapter-specific layouts, this API breaks. The design doc says adapters/projections should be the abstraction boundary; this code punches through it. Severity: [warning]

  5. src/synix/sdk.py: _resolve_flat_file_path() strips output_path to basename
    Path(output_path).name silently discards subdirectories. Two flat files with the same basename collide; nested adapter outputs cannot be addressed. Hidden file-layout coupling. Severity: [warning]

  6. src/synix/sdk.py: Release.close() / scratch releases are not concurrency-safe
    Scratch names are random, but there’s no locking around .synix/work or .synix/releases/_scratch_*. Multiple processes can materialize/clean simultaneously; cleanup can race reads. This matters because the SDK explicitly encourages embedding in runtimes/benchmarks. Severity: [warning]

  7. src/synix/sdk.py: Project.clean() recursively deletes releases/ and work/ with no guardrails
    Public SDK now exposes destructive cleanup with no confirmation, no selective target, no handling for active release handles. That is regression-prone if callers keep Release objects alive. Severity: [minor]

  8. tests/e2e/test_sdk.py validates mostly happy-path behavior, not compatibility/error boundaries
    No tests for malformed receipts beyond one codepath, missing/corrupt search.db, invalid projection receipts, duplicate flat-file basenames, concurrent scratch releases, or project init on nonexistent root path. Given this is a new public API, that test gap is material. Severity: [warning]

Missing

  • Documentation updates to README.md and website content for this major new public capability. A new SDK is user-facing; hiding it only in docs/sdk.md is inconsistent.
  • Explicit “experimental” status for the SDK. Pre-1.0 is not enough when you’re exporting from package root.
  • Tests for init() on a nonexistent target directory, open_project() on a file path, concurrent HEAD scratch usage, cleanup while a release handle is open, and adapter layout changes.
  • Validation that source.add() handles symlinks, overwrite collisions, and large files intentionally.
  • Any migration/compat note for introducing package-root names that may collide with future API cleanup.

VerdictBlock: the SDK is too big a public surface to merge with doc/API mismatches, design-principle drift (build_dir/layout leakage), and missing boundary/concurrency tests.

Review parameters
  • Model: gpt-5.4
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 2,724 lines
  • Prompt: .github/prompts/openai_review.md
  • Timestamp: 2026-03-09T21:55:23Z

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026

Note

Architectural Review — Claude Opus | Blind review (docs + diff only)

Summary

This PR introduces a Python SDK (synix.sdk) that provides programmatic access to Synix projects — init, build, release, search, artifact inspection, source management, and lineage. It's the "boto3 for synix" layer, wrapping existing internals (SnapshotView, release engine, search retriever) into an ergonomic API. The PR includes the implementation, two design docs, and ~1300 lines of e2e tests.

Alignment

Strong fit. DESIGN.md §4.1 explicitly chose Python-first over config, and the SDK extends that principle from pipeline definition to pipeline consumption. The read path goes through releases (immutable snapshots), preserving the build/release separation from §1.5. Artifacts remain immutable and content-addressed — SdkArtifact is a frozen dataclass. Provenance chains are exposed via release.lineage(). The "architecture is a runtime concern" hypothesis (§1.1 H3) is directly served by making it trivial to programmatically swap pipelines and rebuild. The v2 buffer sketch in the design doc (stateful artifacts with instant writes) is deferred but designed-for, which matches the project's incremental philosophy.

Observations

  1. [positive] sdk.py — Fail-closed embedding enforcement is well-designed. The three-way check (declared modes vs. requested mode vs. embedding existence) prevents silent degradation, which matters for the audit-determinism principle.

  2. [positive] Test coverage is excellent. ~850 lines of e2e tests covering: init/discovery, source management (including path traversal rejection), artifact access, keyword/semantic/hybrid/layered search, fail-closed embedding behavior, multi-surface disambiguation, scratch releases with cleanup, incremental rebuilds with cache verification, content-addressed identity across rebuilds, source removal, lineage, refs, clean. Both happy paths and edge cases are covered.

  3. [concern] SdkSource._validate_name checks resolved.parent == self._dir.resolve() — this rejects subdirectories, but shutil.copy2 in add() uses src.name which could collide silently if two files from different directories share a name. No overwrite warning or error.

  4. [concern] Release._get_closure reads receipt.json and extracts snapshot_oid to avoid races, but _resolve_search_projection reads from the closure's projections while search() reads search.db from the release dir. If the release dir is partially written (concurrent release_to), the closure and the db could be inconsistent. The thread-safety note in docs/sdk.md covers threads but not concurrent processes.

  5. [question] Project.build() does copy.deepcopy(original) on the pipeline to avoid mutation. Is deepcopy safe for all layer types? Layers with file handles, compiled prompts, or closures could break. Worth a comment or a test with a non-trivial pipeline.

  6. [nit] docs/sdk.md documents synix.open() as a deprecated alias, but neither sdk.py nor __init__.py actually defines open(). Either add the alias or remove the deprecation note.

  7. [nit] sdk.py:_validate_name — the variable name l in declared = [l.name for l in ...] inside Project.source() shadows the loop variable convention and triggers linter warnings. Use layer instead.

  8. [concern] Project.clean() does shutil.rmtree(releases_dir) unconditionally. If a Release handle is still open against that directory, subsequent reads will fail with confusing filesystem errors rather than a clean SDK error.

  9. [positive] The incremental build test (test_rebuild_no_changes_fully_cached) verifies both zero LLM calls and content-addressed identity (artifact IDs match across builds). This directly validates the materialization key design from DESIGN.md §3.3.

  10. [question] SDK_VERSION = "0.1.0" with independent semver — the docs say major bumps for breaking changes, but since the SDK is re-exported from synix.__init__, won't SDK breaking changes effectively be package breaking changes? The independence claim seems fragile.

Verdict

This is a well-executed incremental step — it makes Synix embeddable in agent runtimes (the primary use case) without introducing new concepts, respects immutability and provenance throughout, and ships with thorough e2e coverage.

Review parameters
  • Model: claude-opus-4-6
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 2,724 lines
  • Prompt: .github/prompts/claude_review.md
  • Timestamp: 2026-03-09T21:55:37Z

- Export full error hierarchy from __init__.py (SynixNotFoundError,
  ReleaseNotFoundError, ArtifactNotFoundError, SearchNotAvailableError,
  EmbeddingRequiredError, PipelineRequiredError, ProjectionNotFoundError)
- Remove stale deprecated-alias note from sdk.md (open was already removed)
- Update sdk.md error import example to use `from synix import` (not sdk)
- Fix variable name `l` → `layer` in list comprehension
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026

Note

Red Team Review — OpenAI GPT-5.4 | Adversarial review (docs + diff only)

Threat assessment — Medium-high risk: this adds a new public SDK surface and exports it from synix.__init__, which is a one-way-door API decision, and the implementation makes some shaky assumptions about release layout and semantics.

One-way doors

  1. Top-level public SDK API in src/synix/__init__.py
    Hard to reverse because open_project, init, Project, Release, exception names, and dataclass fields become user code dependencies immediately. Safe only if the maintainers are willing to support this API through the rest of v0.x with migration notes, or keep it behind an experimental namespace instead of top-level exports.

  2. Project.release("HEAD") as implicit scratch-realization semantics
    Hard to reverse because users will build workflows around HEAD meaning “materialize a temporary release and query it.” Safe only if this exact behavior is documented as experimental and the storage/cleanup contract is tested against concurrent use and failure cases.

  3. SDK error taxonomy
    Exception names and meanings are API contract. Safe only if errors map cleanly to stable failure classes and don’t leak implementation quirks (SearchNotAvailableError vs EmbeddingRequiredError is already fuzzy).

Findings

  1. src/synix/sdk.py / Release.search() hardcodes release_dir / "search.db"
    This ignores the design/docs claim that projections are adapter-driven and extensible. If multiple synix_search projections exist, or adapters write to distinct paths, this will read the wrong DB or fail spuriously. Severity: [critical]

  2. src/synix/sdk.py / _load_embedding_provider() checks only release_dir / "embeddings/manifest.json"
    Same abstraction leak: embeddings are treated as a fixed filesystem convention, not adapter-owned output. This couples SDK behavior to one current release layout and breaks extensibility promised in docs/sdk-design.md. Severity: [warning]

  3. src/synix/sdk.py / Release._release_dir() executes release as a side effect of a read handle
    project.release("HEAD") mutates disk state under .synix/work and .synix/releases during what looks like a read operation. That’s hidden complexity and a race hazard if multiple processes do this simultaneously. No locking is visible. Severity: [warning]

  4. src/synix/sdk.py / Project.clean() recursively deletes .synix/releases and .synix/work with no guardrails
    This is user-facing destructive behavior added to the SDK, but there’s no dry-run, no validation that paths are inside project root, and no concurrency protection if another process is releasing/building. Severity: [warning]

  5. src/synix/sdk.py / BuildResult.cost_estimate documented but absent in code
    docs/sdk-design.md and docs/sdk.md promise cost_estimate/plan-oriented fields, but BuildResult only has built/cached/skipped/time/snapshot_oid/manifest_oid. This is a straight doc/code mismatch in a brand-new API. Severity: [warning]

  6. docs/sdk.md vs implementation naming inconsistency (open_project vs open)
    Docs show project = synix.open("./workspace"), but code exports only open_project. This is exactly the kind of naming drift that becomes permanent once published. Severity: [minor]

  7. docs/sdk.md advertises semver independent of package version via SDK_VERSION
    Shipping a separate version string inside the same package is a maintenance trap. Users will ask which compatibility contract wins: package version or SDK version. No enforcement or policy exists in code. Severity: [minor]

  8. tests/e2e/test_sdk.py reaches into private state (src._dir, release._scratch_dir)
    The tests validate internals rather than public behavior, which means refactors will be painful and they won’t catch actual public API regressions well. Severity: [minor]

Missing

  • No concurrency/locking tests for simultaneous release("HEAD"), release_to(), or clean() calls against the same .synix directory.
  • No failure-path tests for partial scratch release materialization, malformed search.db, failed execute_release, or interrupted cleanup.
  • No docs update in README or website content despite exporting a major new user-facing SDK from top-level package API.
  • No validation around init() on non-empty dirs / existing .synix; expected explicit behavior for overwrite/re-init.
  • No tests for large artifact iteration/search result pagination; current SDK eagerly materializes lists in several places and may not scale cleanly.

VerdictBlock: this is a substantial public API addition with real one-way-door impact, and the implementation is too coupled to current release file layout and under-specified for concurrency and adapter extensibility.

Review parameters
  • Model: gpt-5.4
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 2,729 lines
  • Prompt: .github/prompts/openai_review.md
  • Timestamp: 2026-03-09T22:01:17Z

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026

Note

Architectural Review — Claude Opus | Blind review (docs + diff only)

Summary

This PR introduces a Python SDK (synix.sdk) that provides programmatic access to Synix projects — init, build, release, search, artifact inspection, source management, and lineage. It's the "boto3 for synix" layer, wrapping existing internals behind an ergonomic API. Accompanied by a design doc, user-facing docs, and ~1,300 lines of e2e tests.

Alignment

Strong fit. DESIGN.md §4.1 explicitly chose Python-first over config, and the SDK extends that bet to runtime consumption — agents can now embed Synix directly rather than shelling out. The build/release separation (DESIGN.md §1.5) is respected: all data-plane reads go through Release, which resolves from a specific snapshot OID. Artifacts remain immutable and content-addressed; the SDK only wraps read paths. The ReleaseClosure.from_snapshot pinning in _get_closure() correctly captures the "releases are the read boundary" principle. The v2 buffer sketch in the design doc wisely defers stateful writes while designing for them.

Observations

  1. [positive] Fail-closed embedding enforcement in search() is well-designed — three distinct error paths for missing embeddings, undeclared modes, and missing search.db. Tests cover all three.

  2. [positive] Path traversal protection in SdkSource._validate_name() with corresponding tests. Good security hygiene for a filesystem-touching API.

  3. [concern] _validate_name checks resolved.parent == self._dir.resolve() — this rejects subdirectory paths but also rejects filenames that happen to contain special characters that resolve differently. More importantly, add() uses src.name to extract the filename, so src.add("/etc/passwd") copies it as passwd — the validation protects against add_text("x", "../escape") but add(Path("../../etc/shadow")) would pass validation (the .name is shadow, which resolves inside the dir). Confirm this is intentional.

  4. [concern] Release._release_dir() for scratch creates a UUID-based work dir and calls execute_release as a side effect of a directory accessor. This makes _release_dir() non-idempotent on first call and couples directory resolution with materialization. If execute_release fails, the scratch dir exists but is empty — subsequent calls skip re-materialization because _scratch_dir is not None.

  5. [question] sdk.py line ~430: _get_closure reads snapshot_oid from receipt.json to avoid races. Good. But for scratch releases, _receipt_dir() points to .synix/releases/_scratch_{id}/, which is created by execute_release. If execute_release doesn't write a receipt there (e.g., it writes to the target dir instead), this would fail silently. Is the receipt always written to the refs-based path, not the target path?

  6. [concern] Project.refs() hardcodes three prefixes (refs/heads, refs/releases, refs/runs). If new ref namespaces are added (e.g., refs/batch), this silently omits them. A glob or iter_all_refs() would be more future-proof.

  7. [nit] SdkLayer exposes name, level, count as bare attributes (not properties), making it mutable despite being a read-only view. Consider @dataclass(frozen=True) for consistency with SdkArtifact.

  8. [positive] The incremental build tests (test_sdk_incremental.py) are excellent — they exercise the full lifecycle: initial build → add source → incremental rebuild → verify cache hits → verify new artifacts propagate. The "rebuild no changes fully cached" test with content-addressed identity assertions is exactly what the cache-key model demands.

  9. [question] docs/sdk.md shows project.release("HEAD") as a context manager but also as a bare call. The non-context-manager path requires explicit close() — if forgotten, scratch dirs leak under .synix/work/. Should __del__ be a safety net?

  10. [nit] docs/sdk.md uses synix.open("./workspace") in one example but the actual API is synix.open_project(). Inconsistency will confuse users.

  11. [positive] The build() method deep-copies the pipeline before mutating paths/config, protecting callers from side effects. Good defensive practice.

  12. [concern] clean() does shutil.rmtree on releases/ and work/ with no confirmation or dry-run option. For an SDK call (vs. CLI with --yes), this is fine, but worth noting for scale — cleaning 50 releases is irreversible.

Verdict

This is a well-structured, well-tested incremental step that makes Synix directly embeddable in agent runtimes — a natural progression from CLI-only access that advances the core vision.

Review parameters
  • Model: claude-opus-4-6
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 2,729 lines
  • Prompt: .github/prompts/claude_review.md
  • Timestamp: 2026-03-09T22:01:35Z

- Fix stale synix.open() → open_project() in sdk.md and sdk-design.md
- Fix incorrect BuildResult attributes in sdk-design.md (layers_built,
  cost_estimate → built, total_time, snapshot_oid)
- Fix incorrect release_to() return type in sdk-design.md (dict, not object)
- Document path traversal validation in SdkSource
- Document build() deep-copy behavior
- Update CLAUDE.md module comment to open_project
- Add SDK link to README Learn More table
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026

Note

Red Team Review — OpenAI GPT-5.4 | Adversarial review (docs + diff only)

Threat assessment — Medium-high risk: this PR introduces a new public SDK surface in one shot, wired directly to internals and documented as if stable, but the behavioral contract is underspecified and partly inconsistent with the project’s own design.

One-way doors

  1. Top-level public SDK API (synix.open_project, synix.init, Project, Release, error classes)
    Hard to reverse because it is exported from src/synix/__init__.py and documented in README/docs as first-class. Users will code against these names immediately.
    Safe only if the team is willing to support this API shape through the rest of v0.x or clearly mark it experimental/private.

  2. SDK-specific semantic model around “release(HEAD)” scratch realization
    Hard to reverse because it defines a user mental model and lifecycle contract for ephemeral reads.
    Safe only if scratch release semantics are fully aligned with release engine behavior, cleanup guarantees, and concurrency expectations.

  3. Independent SDK_VERSION semver contract
    Hard to reverse because docs promise versioning separate from package version. That creates a second compatibility surface to maintain.
    Safe only if there is an actual policy/process for dual-version compatibility.

Findings

  1. src/synix/__init__.py + docs/sdk.md: broad export of unstable SDK
    This makes the SDK part of the public package API immediately, but the docs present nontrivial guarantees (“CLI ↔ SDK coverage”, independent semver, error hierarchy) without any experimental caveat. For a pre-1.0 project actively changing storage/release semantics, this is a one-way door.
    Severity: [warning]

  2. src/synix/sdk.py: Release.search() mode contract conflicts with existing docs
    README/public docs use fulltext; SDK docs introduce keyword; website mentions hybrid/semantic; code maps keyword-only by special-casing non-semantic modes but never validates mode names. This fractures the public search vocabulary and will leak adapter internals into user code.
    Severity: [warning]

  3. src/synix/sdk.py: Release.search() imports CLI module (synix.cli.search_commands.ReleaseProvenanceProvider)
    SDK depends on CLI-layer code. That is upside-down architecture and a direct abstraction leak. Refactoring CLI internals later will break SDK consumers.
    Severity: [critical]

  4. src/synix/sdk.py: init() still creates/uses build_dir concepts indirectly
    README/design say .synix/ is single source of truth and “there is no build/ directory.” Yet SDK docs and tests still encourage build_dir="./build" and path resolution mutates it. That’s a design-doc mismatch and regression risk toward legacy semantics.
    Severity: [warning]

  5. src/synix/sdk.py: Project.build() mutates pipeline semantics via path rewriting
    It deep-copies, then rewrites source_dir, build_dir, and per-source dir to absolute paths because runner resolves against CWD. That means SDK correctness depends on a known runner bug/assumption rather than fixing the boundary once. Hidden complexity: fingerprints or serialization could accidentally depend on path form.
    Severity: [warning]

  6. src/synix/sdk.py: Release.close() uses unconditional shutil.rmtree() on shared work/release paths
    No locking, no ownership marker beyond generated names, no race protection. Multiple processes using .synix/work or scratch releases can delete each other’s state.
    Severity: [warning]

  7. src/synix/sdk.py: SdkArtifact.provenance / Release.lineage() semantics are muddled
    Docs claim provenance is “parent labels” in design, SDK docs say “BFS-walked parent labels,” and tests expect the artifact itself to appear in provenance/lineage. That is not a crisp provenance contract and will confuse downstream tooling.
    Severity: [warning]

  8. docs/sdk.md: source management promises remove() but docs/sdk-design.md public API omits it
    Documentation is internally inconsistent. Same for Project.clean()—documented in coverage table but omitted from the API surface section in design doc.
    Severity: [minor]

  9. tests/e2e/test_sdk.py and incremental tests cover happy paths but not failure boundaries
    There’s no test for malformed receipt.json, invalid release refs in release_to(), duplicate init() into existing non-empty dirs, missing source file on SdkSource.add(), permission errors, or concurrent scratch release access.
    Severity: [warning]

Missing

  • Clear “experimental SDK” labeling in README/docs, given this is a brand-new public API.
  • Validation of mode values in Release.search() with a canonical vocabulary aligned to README/website.
  • Tests for concurrent scratch releases / multiple SDK handles against one .synix/.
  • Tests for malformed receipts and partial releases.
  • Documentation update reconciling SDK examples with the no-build/-directory design.
  • Any compatibility statement for how SDK behavior relates to future on-disk format changes.

VerdictBlock: this is not just additive docs; it cements a public API with architectural layering problems and inconsistent user-facing semantics.

Review parameters
  • Model: gpt-5.4
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 2,744 lines
  • Prompt: .github/prompts/openai_review.md
  • Timestamp: 2026-03-09T22:18:50Z

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026

Note

Architectural Review — Claude Opus | Blind review (docs + diff only)

Summary

This PR adds a Python SDK (synix.sdk) providing programmatic access to the full synix lifecycle — init, build, release, search, inspect — without shelling out to the CLI. It includes the implementation module, a design doc, a user-facing guide, and ~1,300 lines of e2e tests covering both unit-style and incremental pipeline scenarios.

Alignment

Strong fit. DESIGN.md's Python-first decision (§4.1) explicitly argues that the target user is already writing Python and the interface should be composable with agent code. The SDK directly enables the two use cases the design anticipates: agent runtime integration (core memory in system prompts, RAG tool) and benchmark harnesses. The read boundary being releases (§1.5 build/release separation) is faithfully preserved — all data-plane reads go through Release, which resolves from a receipt-pinned snapshot OID. Artifacts remain immutable and content-addressed; the SDK wraps SnapshotView/ObjectStore without duplicating storage. Provenance chains are exposed via lineage() and provenance on every SdkArtifact. The v2 buffer sketch in the design doc is deferred cleanly without polluting v1 contracts.

Observations

  1. [positive] sdk.py:_validate_name rejects path traversal (../, nested paths) on source operations, with tests for both add_text and remove. Good security hygiene for a file-managing API.

  2. [positive] build() deep-copies the pipeline before mutating paths/config (sdk.py:~570), protecting the caller's object. Explicitly documented and tested.

  3. [concern] SdkSource.add() uses src.name as the destination filename (sdk.py:208). If two different source files have the same basename, the second silently overwrites the first. No warning or error.

  4. [concern] Release._get_closure() reads receipt.json and extracts snapshot_oid to avoid races — good. But _release_dir() for scratch releases calls execute_release as a side effect of a property-like method. If _release_dir() is called twice (e.g., search then flat_file), the guard if self._scratch_dir is None prevents double-materialization, but the coupling between "get a path" and "materialize a release" is surprising.

  5. [question] Release.search() instantiates a new SearchIndex and HybridRetriever on every call (sdk.py:~430). For a tight loop (benchmark with 1,000 queries), this could be expensive — repeated SQLite opens, embedding model loads. Is caching these handles on the Release instance planned?

  6. [nit] SdkLayer exposes name, level, count as bare attributes (not properties), unlike the rest of the SDK which uses @property or frozen dataclasses. Minor inconsistency.

  7. [positive] The fail-closed embedding enforcement is well-designed: declared semantic capability without materialized embeddings is a hard error, not a silent fallback to keyword. Tests cover all four quadrants (declared+present, declared+missing, undeclared+requested, keyword-only).

  8. [positive] test_sdk_incremental.py is excellent — it exercises the full init→build→release→search→add-source→rebuild→re-release cycle, validates cache hits (calls_build2 == 0), content-addressed identity across rebuilds, source removal, and ref tracking. This is the kind of e2e coverage that catches real integration bugs.

  9. [question] Project.clean() does shutil.rmtree on releases/ and work/. The README says clean "does not delete snapshot history," which aligns. But the SDK doc doesn't mention this — a user calling project.clean() might expect it to also clean objects.

  10. [concern] The __init__.py re-exports create a name collision: both synix.core.models and synix.sdk export ArtifactNotFoundError (the existing SearchSurfaceLookupError etc. are already exported above). If the original __init__.py already exports an ArtifactNotFoundError from another module, this would shadow it. Worth verifying.

  11. [nit] sdk.py imports logging and creates a logger but never uses it in the diff. Dead import.

Verdict

This is a well-designed, well-tested incremental step that makes synix embeddable as a library — directly advancing the Python-first and build/release separation principles from DESIGN.md.

Review parameters
  • Model: claude-opus-4-6
  • Context: README.md, DESIGN.md, synix.dev, PR diff
  • Diff size: 2,744 lines
  • Prompt: .github/prompts/claude_review.md
  • Timestamp: 2026-03-09T22:19:08Z

@marklubin marklubin merged commit 9ec685c into main Mar 9, 2026
12 checks passed
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.

Rich search output: content + provenance + metadata in one call

1 participant