schema: close the post-split audit gaps - #403
Merged
Merged
Conversation
- unreachable_from joins ADR 0009 §6.3's traversal exclusion via the
new Context::unreachable_from_excluding (explore_excluding's
monomorphized-closure pattern): once a schema exists, schema:type
edges are never a bridge in the coverage audit's walk and never
reported as orphans, so a shared type name can no longer hide
genuine orphans; regression test pins both halves of the flip.
- Both core SDKs surface §8.3's warn-mode carrier instead of stripping
it with the envelope: add_associations returns
AddAssociationsResult {applied, issues, schema_violations} (breaking;
pre-1.0), BatchApplyResult aggregates per chunk, ImportResult gains
schemas/issues/schema_violations. Plumbed through the helper stack
(_request_json_full/_post_full, requestJsonFull/postFull) beside the
existing result-only path, never around it.
- SDK schema surface parity with HTTP/MCP: put_schema, audit_schema,
validate_schema in Python and TypeScript, decoding the shared
SchemaAudit shape; recorded in sdk/spec/surface.yaml.
- Live protocol manual catches up: the four schema routes (plus the
pre-existing /drift/audit gap), schema_mode on GET /contexts' row
shape, and no_schema in the stable error-code vocabulary.
- Env-var docs: the two TAGURU_MCP_* knobs land in README, four
missing KNOWN_KEYS land in getting-started's table; python-langchain's
stale "PROMPT_VERSION 2" comment now matches its constant and its
TypeScript twin.
Closes #402
Claude-Session: https://claude.ai/code/session_01HqB7fXgCKSnDxT58PenLaS
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughスキーマ登録・監査・検証APIをPython/TypeScript SDKへ追加しました。警告情報とスキーマ違反数をSDK結果へ引き継ぎます。 Changesスキーマ監査と到達性制御、プロトコル記載
SDKモデルと応答エンベロープ
SDKスキーマ操作と関連追加結果
SDKのテスト検証
文書と設定情報
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SDK as SDK Client
participant Context
participant Transport as _post_full
participant API as HTTP API
SDK->>Context: put_schema、audit_schema、validate_schema
Context->>Transport: リクエスト＋完全エンベロープ取得
Transport->>API: スキーマAPIへ送信
API-->>Transport: result + issues + schema_violations
Transport-->>Context: 構造化結果
Context-->>SDK: SchemaDocument / SchemaAudit
SDK->>Context: add_associations(associations)
Context->>Transport: 完全エンベロープで送信
Transport->>API: 関連追加へPOST
API-->>Transport: applied + issues + schema_violations
Transport-->>Context: AddAssociationsResult
Context-->>SDK: {applied, issues, schema_violations}
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/api/coverage.rs (1)
257-264: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winスキーマなしの監査では非除外経路を使用してください。
Line 258 で
hiddenがない場合も、空のexcludedをunreachable_from_excludingに渡しています。これにより、全エッジ走査でHashSet::containsが実行されます。unreachable_fromはこの判定を除くため、スキーマなしの大きなコンテキストでは不要な CPU コストになります。
excluded.is_empty()の場合はunreachable_fromを呼び、ラベルがある場合だけ除外版を呼んでください。修正案
let origins: Vec<&str> = request.origins.iter().map(String::as_str).collect(); - context - .unreachable_from_excluding(&origins, deadline, &excluded) - .map_err(|_| AccessError::DeadlineExceeded) + let result = if excluded.is_empty() { + context.unreachable_from(&origins, deadline) + } else { + context.unreachable_from_excluding(&origins, deadline, &excluded) + }; + result.map_err(|_| AccessError::DeadlineExceeded)🤖 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 `@src/api/coverage.rs` around lines 257 - 264, Update the loaded-context traversal in read_context to call unreachable_from when excluded is empty, and call unreachable_from_excluding only when hidden labels produced exclusions. Preserve the existing origins and deadline arguments in both paths.
🤖 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 `@sdk/python/tests/unit/test_get_schema.py`:
- Around line 117-140: Extend sdk/python/tests/unit/test_get_schema.py lines
117-140 and sdk/typescript/tests/unit/get-schema.test.ts lines 95-117 to pass
the same non-trivial MatchCursor to audit_schema/auditSchema and
validate_schema/validateSchema, then assert each request body’s after field
exactly matches the cursor’s expected JSON wire representation while retaining
the existing limit and response assertions.
In `@sdk/python/tests/unit/test_pagination_and_batching.py`:
- Around line 162-196: Update
sdk/python/tests/unit/test_pagination_and_batching.py lines 162-196 so the
handler returns chunk-specific Issue.path values and schema_violations counts,
then assert batched issues retain chunk order and the aggregate violation total.
Add an addAssociationsBatched warn-mode test in
sdk/typescript/tests/unit/transport.test.ts lines 384-404 using two distinct
envelopes, asserting issue ordering and summed schema violations.
In `@src/llm-protocol.md`:
- Around line 397-399: Update the documentation for the async client’s
get_schema method to use NotFoundError.code and distinguish no_schema from
no_context without requiring an additional list or get request. Then regenerate
the generated synchronous client with scripts/generate_sync.py; do not edit the
_sync implementation directly.
In `@tests/http_api/schema_type_label.rs`:
- Around line 271-279: The assertion on line 278 only verifies the subject field
of the first match, leaving the label and object fields unvalidated. Expand the
assertion to also verify that after["matches"][0]["label"] equals "schema:type"
and after["matches"][0]["object"] equals "Brewery", ensuring the complete edge
structure is validated and not just a single property that happens to pass by
coincidence.
---
Nitpick comments:
In `@src/api/coverage.rs`:
- Around line 257-264: Update the loaded-context traversal in read_context to
call unreachable_from when excluded is empty, and call
unreachable_from_excluding only when hidden labels produced exclusions. Preserve
the existing origins and deadline arguments in both paths.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: dcff915e-0ea8-4b75-beda-ca6d6b779f17
📒 Files selected for processing (27)
CHANGELOG.mdREADME.mddocs/getting-started.htmldocs/schema.htmlsdk/python-langchain/src/taguru_langchain/_extract.pysdk/python/src/taguru/__init__.pysdk/python/src/taguru/_async/client.pysdk/python/src/taguru/_models.pysdk/python/src/taguru/_shared.pysdk/python/src/taguru/_sync/client.pysdk/python/tests/integration/test_full_loop.pysdk/python/tests/unit/test_get_schema.pysdk/python/tests/unit/test_pagination_and_batching.pysdk/python/tests/unit/test_retry.pysdk/spec/surface.yamlsdk/typescript/src/client.tssdk/typescript/src/index.tssdk/typescript/src/models.tssdk/typescript/src/transport.tssdk/typescript/tests/integration/client.test.tssdk/typescript/tests/unit/get-schema.test.tssdk/typescript/tests/unit/retry.test.tssdk/typescript/tests/unit/transport.test.tssrc/api/coverage.rssrc/context/traverse.rssrc/llm-protocol.mdtests/http_api/schema_type_label.rs
- Pin the reported orphan's whole edge (label/object, not subject alone) in the coverage-audit exclusion test. - Verify audit_schema/validate_schema's `after` cursor reaches the wire verbatim in both SDK unit suites. - Batched warn-carrier aggregation now answers DISTINCT per-chunk issues/counts (order + sum visible), and the TypeScript suite gains the batched warn test it lacked. - get_schema's doc now points at the no_schema/no_context error codes instead of claiming a second request is needed; sync regenerated. Skipped the coverage.rs is_empty() branch nitpick: explore/activate pass a possibly-empty exclusion list unconditionally, and diverging in one handler would trade a per-edge empty-HashSet probe for an inconsistency with the established pattern. Claude-Session: https://claude.ai/code/session_01HqB7fXgCKSnDxT58PenLaS
Owner
Author
|
Addressed the CodeRabbit review in 0d679be:
Skipped the |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@sdk/typescript/tests/unit/transport.test.ts`:
- Around line 410-426: Update the stubClient handler around issueFor and the
response body so it parses each request body, identifies whether it contains
op(0) or op(1), and derives issues and schema_violations from that input
identifier rather than chunksSeen call order. Keep the expected values tied to
the corresponding input chunk so the test detects reordered chunk transmission.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5f254785-b31d-4ce1-a5ae-9354e8570993
📒 Files selected for processing (7)
sdk/python/src/taguru/_async/client.pysdk/python/src/taguru/_sync/client.pysdk/python/tests/unit/test_get_schema.pysdk/python/tests/unit/test_pagination_and_batching.pysdk/typescript/tests/unit/get-schema.test.tssdk/typescript/tests/unit/transport.test.tstests/http_api/schema_type_label.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- sdk/python/tests/unit/test_pagination_and_batching.py
- tests/http_api/schema_type_label.rs
- sdk/typescript/tests/unit/get-schema.test.ts
- sdk/python/tests/unit/test_get_schema.py
- sdk/python/src/taguru/_async/client.py
- sdk/python/src/taguru/_sync/client.py
The batched warn-carrier stubs now derive each response from the chunk the request actually carried (parsed from the body's op subject), not from handler call order — a reordered transmission, a dropped chunk, or a double-count are all now distinguishable. Applied to the Python twin too, which had the same call-order weakness the review caught on the TypeScript side. Claude-Session: https://claude.ai/code/session_01HqB7fXgCKSnDxT58PenLaS
Owner
Author
|
Addressed the round-2 comment in 9b647d9: the batched warn-carrier stubs now parse each request body and derive |
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
1 participant
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
A full audit of the unreleased v0.6.0..HEAD range (the #218/ADR 0009 schema split) found no logic bugs, but five gaps between what shipped and what the ADR/docs promise. This PR closes all of them:
unreachable_fromjoins ADR 0009 §6.3's traversal exclusion. New additiveContext::unreachable_from_excluding(same monomorphized-visible-closure pattern asexplore_excluding, so the unfiltered path pays nothing); the handler resolveshidden_labelbeforeread_contextper the documented deadlock rule. Hidden edges are "never a bridge, never reported" — a shared type name can no longer make genuine orphans look covered. Regression test pins the before/after flip intests/http_api/schema_type_label.rs.add_associations/addAssociationsreturnsAddAssociationsResult {applied, issues, schema_violations};BatchApplyResultaggregates per chunk;ImportResultgainsschemas/issues/schema_violations. Wired through new_request_json_full/_post_full(Python) andrequestJsonFull/postFull(TS) beside the existing result-only helpers. Migration:.applied; callers ignoring the return value are unaffected.put_schema/audit_schema/validate_schemain both SDKs, decoding the sharedSchemaAuditshape; recorded insdk/spec/surface.yaml(both surface checks pass).GET /protocol/MCP instructions) catches up: the four schema routes plus the pre-existing/drift/auditgap,schema_modeonGET /contexts' row shape,no_schemain the error-code vocabulary.TAGURU_MCP_*knobs in README, four missingKNOWN_KEYSin getting-started's env table (all 60 keys now cross-checked), stalePROMPT_VERSION 2comment fixed in python-langchain.Closes #402
Test plan
cargo fmt --check/cargo clippy --all-targets(zero warnings)cargo test— full suite green, including the new coverage-audit regression test (http_api 453)ruff format --check/ruff check/mypy --strict src/pytest198 passed (integration spawns the real server binary) /check_surface.pytsc --noEmit/eslint/vitest186 passed (integration included) /check-surface.ts/tsupbuildhttps://claude.ai/code/session_01HqB7fXgCKSnDxT58PenLaS
Summary by CodeRabbit