schema: POST /contexts/{name}/schema/audit and /schema/validate - #398
Merged
Conversation
Adds the read-only standing audit and never-persisted dry-run over ADR 0009's schema document (#385, S7 of #218's split §10). Both share one judgment built on the existing schema_issues/SchemaEnv pure check (S3, #381) so a finding here is exactly what strict would refuse for the same fact, and both judge mode-independently — pre-existing violations are otherwise invisible once a context sits in off/warn. Claude-Session: https://claude.ai/code/session_01FJSVDt6KNgjbjdtJeLxMC9
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 (7)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughスキーマ監査APIと提案スキーマ検証APIを追加しました。共通検査エンジンがライブ関連付けと提案文書を評価します。HTTPルート、Read権限、heavy-operation limiter、MCPツール、統合テストを追加しました。 Changesスキーマ監査と検証
Estimated code review effort: 4 (Complex) | ~50 minutes Possibly related issues
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: 9
🧹 Nitpick comments (3)
src/api/schema.rs (2)
239-247: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
reserved_alias_conflictsだけが上限を持ちません。
untyped_concepts、undeclared_types、unknown_labelsはMAX_AUDIT_NAMESで上限を持ちます。reserved_alias_conflictsはライブのlabel_aliases()の絞り込み結果をそのまま返します。件数に上限がありません。
validate_schemaはスキーマ未インストールのコンテキストにも適用できます。そのコンテキストではPUT /schemaの install 時ガードが一度も動いていません。予約ラベルへ解決するエイリアスが多数存在すると、レスポンスが非有界に大きくなります。他の3セクションと同じく
AuditNamesへ揃えるか、上限を持たない理由を doc コメントに追記してください。🤖 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/schema.rs` around lines 239 - 247, Align reserved_alias_conflicts with the bounded audit sections by collecting it through AuditNames and enforcing MAX_AUDIT_NAMES while filtering label_aliases() for aliases resolving to schema:type. Preserve the existing alias-to-canonical mapping and response behavior for retained entries.
329-350: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
fact_edgesとfact_opsが同じ文字列を二重に保持します。ループは各 fact エッジについて
subject、label、objectをAssocOpへ clone し、その後Association本体もfact_edgesへ push します。エッジ数を N とすると、3N 回の追加Stringアロケーションが発生します。この経路は O(edges) の heavy operation であり、大きなコンテキストではこのコピーがピークメモリを押し上げます。
fact_edgesを先に確定し、fact_opsをその借用から組み立てる方法はSchemaEnv::buildの&[AssocOp]シグネチャと衝突します。現状の構造を保つ場合は、少なくともfact_edgesとfact_opsの両方にVec::with_capacityを与えて再アロケーションを減らしてください。🤖 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/schema.rs` around lines 329 - 350, Update the fact collection setup in SchemaEnv::build to initialize both fact_edges and fact_ops with capacity based on the live association count before the loop, while preserving the existing ownership and cloning behavior.src/schema/check.rs (1)
368-400: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
IssuePath::Edgeのパス生成にユニットテストがありません。このモジュールのテストはすべて
IssuePath::Requestを使います。Edgeバリアントがsubject/object/labelの裸のパスを返すことは、ここでは検証されていません。associations_fieldの分岐が壊れても、このモジュールのテストは失敗しません。
domain_violation_is_reported_on_the_subject_pathと対になるEdge版のテストを追加してください。統合テスト側で同じ契約を検証済みであれば、この指摘は不要です。♻️ 追加テストの例
#[test] fn the_edge_path_names_the_side_alone() { let mut context = Context::default(); context .associate_from("山田太郎", SCHEMA_TYPE_LABEL, "Person", 1.0, "a.md", None) .unwrap(); let schema = installed(doc(SchemaMode::Strict, false)); let ops = [assoc_op("山田太郎", "杜氏", "鈴木一郎", 1.0, None)]; let env = env(&context, schema, &ops); let check = schema_issues(&env, &ops, IssuePath::Edge); assert_eq!(check.violations[0].path, "subject"); }🤖 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/schema/check.rs` around lines 368 - 400, Add a unit test alongside domain_violation_is_reported_on_the_subject_path that invokes schema_issues with IssuePath::Edge and verifies the reported violation path is the bare side name, such as "subject". Reuse the existing context, schema, operation, and helper setup patterns, and ensure the test exercises the Edge branch rather than IssuePath::Request.Source: Path instructions
🤖 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 `@CHANGELOG.md`:
- Around line 24-32: Update the changelog sentence describing the DriftAudit
response to say “Five candidates-not-verdicts sections,” matching the five
listed sections: violations, untyped_concepts, undeclared_types, unknown_labels,
and reserved_alias_conflicts.
In `@src/api/schema.rs`:
- Around line 368-387: SchemaEnv::build を呼び出す前に deadline.expired()
を確認し、期限切れなら既存の AccessError::DeadlineExceeded を返すよう更新してください。SchemaEnv::build 後の
violation ループと各エッジのチェックは変更せず、build 前の単一チェックのみ追加してください。
In `@src/main.rs`:
- Around line 781-785: schema/audit と schema/validate の wire
契約を追加し、DriftAudit、ページング、および document・limit・after の形状と転送を固定してください。src/main.rs
のルート登録は維持し、tests/http_api/contract.rs に両 HTTP 操作のプローブを追加し、対応する HTTP/MCP fixture
と shapes.json の必須入力を更新してください。src/mcp/route.rs の該当 MCP ルートおよび src/mcp/schema.rs
の該当スキーマ定義も契約対象として fixture・形状に反映し、互換追加のため契約値は bump しないでください。
In `@src/mcp.rs`:
- Around line 610-617: Extend the MCP routing tests around the cases list to
cover the after parameter for both audit_schema and validate_schema. Add
assertions verifying that route_tool forwards after into the HTTP request body,
rather than only validating the input schema. Keep the existing field checks
unchanged for the other tools.
In `@src/schema.rs`:
- Line 274: InstalledSchema の enforcing を self: &Arc<Self>
のメソッドレシーバではなく、Arc<InstalledSchema> を引数に取る関連関数へ変更してください。あわせて、src/api/schema.rs の
enforcing 呼び出しを新しい関連関数形式に更新し、既存の戻り値と動作を維持してください。
In `@tests/http_api/key_scopes_cross_context.rs`:
- Around line 89-105: Extend the key-scope test around the existing GET /schema
404 assertion to configure a schema using the admin key, then call schema/audit
with the reader key rtok and assert a 200 response. Use the existing call helper
and setup flow, while preserving the current schema/validate reader-key
assertion.
In `@tests/http_api/schema_audit.rs`:
- Around line 109-117: Update tests/http_api/schema_audit.rs lines 109-117 to
store each complete audit response and assert all mode responses are identical,
rather than comparing only total. Update lines 410-416 to compare the complete
audit responses for without and with cases, ensuring every section remains
independent of mode and resident schema effects.
- Around line 1-8: Add schema audit coverage in the tests around schema_audit to
define a deprecated relation whose use would normally produce a domain or range
violation, then assert that it is omitted from both violations and total. Ensure
the test exercises the audit endpoint’s filtering behavior and would fail if
deprecated relations are included again.
- Around line 268-288: Strengthen the pagination assertions in the schema audit
test by validating each violation’s weight and association identity, not only
page lengths. Using the existing first_matches and second_matches results,
assert that 弟子1, 弟子2, and 弟子3 appear in severity order exactly once across both
pages, confirming the cursor after the second first-page result resumes at the
correct item without duplication.
---
Nitpick comments:
In `@src/api/schema.rs`:
- Around line 239-247: Align reserved_alias_conflicts with the bounded audit
sections by collecting it through AuditNames and enforcing MAX_AUDIT_NAMES while
filtering label_aliases() for aliases resolving to schema:type. Preserve the
existing alias-to-canonical mapping and response behavior for retained entries.
- Around line 329-350: Update the fact collection setup in SchemaEnv::build to
initialize both fact_edges and fact_ops with capacity based on the live
association count before the loop, while preserving the existing ownership and
cloning behavior.
In `@src/schema/check.rs`:
- Around line 368-400: Add a unit test alongside
domain_violation_is_reported_on_the_subject_path that invokes schema_issues with
IssuePath::Edge and verifies the reported violation path is the bare side name,
such as "subject". Reuse the existing context, schema, operation, and helper
setup patterns, and ensure the test exercises the Edge branch rather than
IssuePath::Request.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d9839344-0431-485f-9090-10ebf51cffcf
📒 Files selected for processing (19)
CHANGELOG.mdsrc/api.rssrc/api/associations.rssrc/api/schema.rssrc/auth.rssrc/context/query.rssrc/ingest.rssrc/limits.rssrc/main.rssrc/mcp.rssrc/mcp/route.rssrc/mcp/schema.rssrc/schema.rssrc/schema/check.rstests/http_api/key_scopes_cross_context.rstests/http_api/main.rstests/http_api/mcp_basics.rstests/http_api/replication.rstests/http_api/schema_audit.rs
Bounds reserved_alias_conflicts like the other audit sections, recovers enforcing() to a plain associated function instead of an Arc-receiver method, pre-flights the deadline before SchemaEnv::build's second graph read, and pre-sizes the fact_edges/fact_ops buffers. Strengthens the integration tests (full-response mode-invariance and validate parity comparisons, exact pagination identity) and adds the missing IssuePath::Edge/after-forwarding unit test coverage. Skips the "deprecated relation" audit suggestion (no such field exists on the schema document yet, per ADR 0009 §9.2) and the wire-contract fixture suggestion (the existing vocabulary/audit and drift/audit routes carry no such fixture either). Claude-Session: https://claude.ai/code/session_01FJSVDt6KNgjbjdtJeLxMC9
5 tasks
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
POST /contexts/{name}/schema/audit(judges every live association against the resident schema document) andPOST /contexts/{name}/schema/validate(dry-runs a proposed document without persisting it) — S7 of schema: add optional entity types and relation ontology constraints #218's ADR 0009 split, §10.schema_audit,src/api/schema.rs) built on the existingschema_issues/SchemaEnvpure check every write entrance already uses (S3, schema: the reserved type label and the shared pre-write check #381), and both judge asstrictwould regardless of the document's actualmode— per §7.1, pre-existing violations are otherwise invisible inoff/warn.DriftAudit-shaped:violations(domain/range, the only paged section),untyped_concepts,undeclared_types(§6.2, always on),unknown_labels(§6.4, only underclosed_labels), andreserved_alias_conflicts(§6.3 guard 2, only reachable throughvalidatesincePUT /schemaalready refuses to install over such a conflict). Deprecated-relation usage is out of scope per §9.2.Role::Read, join the unconditional heavy-ops group, and haveaudit_schema/validate_schemaMCP tool twins.Test plan
cargo fmt --all --checkcargo clippy --all-targets -- -D warningscargo test(full suite, 2066 tests, 0 failed)tests/http_api/schema_audit.rs(11 cases: 404s, mode-independence, all four audit sections, pagination, reserved-alias conflict viavalidate, invalid-document refusal, non-persistence)key_scopes_cross_context.rs,replication.rs, andmcp_basics.rsfor role classification, replica read-through, and MCP round-tripCloses #385
https://claude.ai/code/session_01FJSVDt6KNgjbjdtJeLxMC9
Summary by CodeRabbit
新機能
テスト