feat: ontology/catalog plane above the document KG (#13) - #73
feat: ontology/catalog plane above the document KG (#13)#73seonghobae wants to merge 17 commits into
Conversation
Add create/list/get/query for glossary and catalog objects that reference naruon document-KG ids instead of ingesting documents. Bind Keyverse OIDC subject and X-CWL-Tenant-Reference fail-closed, keep steward PII usable, and persist the plane in 3NF snake_case tables (issue #13). Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
📝 WalkthroughWalkthrough온톨로지·카탈로그 평면을 추가했다. 테넌트 바인딩, 카탈로그 객체 API, 문서 KG 연결, 개념 바인딩, 검색, 3NF 마이그레이션 및 관련 문서를 구현했다. 테스트는 권한, 격리, JWT, 입력 검증을 확인한다. Changes온톨로지·카탈로그 평면
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟠 High · up to This PR adds externally reachable catalog-object writes and queries, but the current implementation can trust a caller-supplied identity header as an administrator and omits required policy decision and audit propagation; it also permits relationship duplicates and unsafe concurrent access in the in-memory store. These create concrete security, consistency, and availability risks, so the PR is not ready to merge until the identity boundary and enforcement/storage issues are addressed. Sequence Diagram(s)sequenceDiagram
participant Client
participant FastAPI
participant bind_keyverse_tenant
participant catalog_plane
participant CatalogStore
Client->>FastAPI: catalog-plane request with tenant headers
FastAPI->>bind_keyverse_tenant: validate identity, tenant, and purpose
bind_keyverse_tenant-->>FastAPI: return PlaneActor
FastAPI->>catalog_plane: execute catalog operation
catalog_plane->>CatalogStore: create, retrieve, link, or query object
CatalogStore-->>catalog_plane: return tenant-scoped result
catalog_plane-->>FastAPI: return PlaneEnvelope
FastAPI-->>Client: return response or mapped error
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Reject X-CWL-Oidc-Subject when Keyverse JWKS is configured, reuse policy.evaluate for create/search evidence without masking, lock the in-memory store, and enforce unique child keys plus opaque https-only references. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Route create/list/get/query/attach through a 0002-backed store when the DSN is set so a paid-pilot restart keeps glossary and catalog rows. Keep the in-memory store as the CI/pytest default. SQLite unit tests exercise the same table mapping without Apache AGE. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
The test-only 0002 adapter already maps TIMESTAMPTZ to TEXT. SQLite also rejects DEFAULT now(), so unit tests now substitute CURRENT_TIMESTAMP. Production Postgres still uses the original migration. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.
Findings
1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
- Problem: Failed same-head checks remain for
bfa409f8971a3b28aac31ec7bca371fa94f7f07f. - Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
- Fix: Read and fix the failed check logs below, then rerun the current-head checks.
- Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.
Failed checks:
- Security Scan/trivy-fs: FAILURE (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/32126691130/job/95678711818)
- trivy-fs check run: failure (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/32126691130/job/95678711818)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (10 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (10 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (3 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (3 files)"]
R2 --> V2["docs review"]
Evidence --> S3["Test (7 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (7 files)"]
R3 --> V3["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed. Findings1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
Failed checks:
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (10 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (10 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (3 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (3 files)"]
R2 --> V2["docs review"]
Evidence --> S3["Test (7 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (7 files)"]
R3 --> V3["targeted test run"]
|
opencode-agent
Bot
added
priority: medium
|
Caution Review failedAn error occurred during the review process. Please try again later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Warning Review limit reachedNext included review available in 3 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (24)
📝 WalkthroughWalkthrough카탈로그 플레인에 tenant-bound API, 정책 결정 기록, 메모리·관계형 저장소 선택, 0002 마이그레이션, 인증 제한을 추가했다. 기업 마스터 해석의 소유권과 Changes카탈로그 계약과 저장 구조
테넌트 바인딩과 인증 경계
카탈로그 저장소 구현
카탈로그 처리와 API 연결
동작과 영속성 검증
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🔵 Low · up to The PR adds tenant-scoped catalog persistence, querying, and attachment APIs while tightening subject-header authentication defaults. It is mergeable with explicit owner awareness for test isolation and cleanup, managed access to the authentication configuration, SQL query efficiency, and documentation accuracy; the supplied evidence does not show a release-blocking defect. Sequence Diagram(s)sequenceDiagram
participant Client
participant FastAPI
participant TenantBinding
participant CatalogPlane
participant CatalogPlaneStore
Client->>FastAPI: tenant 및 인증 헤더와 catalog 요청
FastAPI->>TenantBinding: identity·tenant·purpose 검증
TenantBinding-->>FastAPI: tenant-bound actor 반환
FastAPI->>CatalogPlane: 카탈로그 작업 호출
CatalogPlane->>CatalogPlaneStore: 객체 저장·조회·연결·검색
CatalogPlaneStore-->>CatalogPlane: 테넌트 범위 레코드 반환
CatalogPlane-->>FastAPI: policy_decision_id 포함 envelope 반환
FastAPI-->>Client: 결과 또는 HTTP 오류 반환
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 93.22% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 118 functions across 12 files. (7 skipped: 7 unsupported.) ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Preserve policy-driven apply_mask (PRD P0); scope no-new-masking rule to the catalog plane; GRC owns redaction/evidence export contract. - Document-KG owner is naruon; LineageWeave owns lineage DAG reconstruction. Portal writes neither; provenance pointers only (PROV-DM row aligned). - Record caller-supplied user on /browse preview as an open gap instead of claiming fail-closed today; DSN does not yet persist catalog CRUD until #73. - Cite existing Figma file JjYSqr6nWxpARUjaVKhG16 from design-tokens contract; Storybook scene/edge-case event inventory listed as open UI gap. - Mark NVIDIA_NIM_API_KEY as external orchestrator connector var, currently unreferenced by this repo's workflows/tests. - Insert PR #81 (CVE-2026-69247 cryptography bump) at top of merge order.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
# Conflicts: # src/sdp/api.py # tests/test_api.py
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
🔍 Verified roles ignored outside create/search branches
evaluate only forwards roles in the early create and search branches. The dataset-level checks (_can_mutate, _is_admin(subject) at policy.py:84 and :92, _has_reader_role(subject) at policy.py:108) drop it and fall back to demo-map has_role. Harmless now since the plane only uses create/search, but a future caller passing verified roles for query/preview/schema/publish would silently be evaluated against demo-map roles instead.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| license: str = "internal" | ||
| related_datasets: list[str] = Field(default_factory=list) | ||
| schema: list[ColumnMetadata] = Field(default_factory=list) | ||
| dataset_schema: list[ColumnMetadata] = Field(default_factory=list, alias="schema") |
There was a problem hiding this comment.
📝 Info: schema field renamed with backward-compatible alias
Dataset and the create/patch requests rename schema to dataset_schema with alias="schema" and serialize_by_alias=True, so JSON input/output keeps the schema key while avoiding the Pydantic .schema() method shadow. All internal .schema reads were updated, demo_seed.py still binds via the alias, and validate_metadata maps the schema requirement to dataset_schema. No stale dataset.schema attribute reads remain in src.
Was this helpful? React with 👍 or 👎 to provide feedback.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| import sdp.catalog as app_catalog | ||
| import sdp.catalog_plane as app_catalog_plane | ||
| import sdp.api as app_api |
2 participants
Closes #13.
Intent
Add the buyer-facing ontology/catalog plane above naruon's document knowledge graph. A steward can register, list, retrieve, and query tenant-scoped glossary terms, catalog datasets, and concept assets while keeping document bodies and specialist-engine responsibilities in their owning products.
This is not LineageWeave, DiskSage ingest/preview, a local GRC registry, or a TEPP/IRT scoring implementation.
Buyer workflow
POST /plane/catalog-objects— register a glossary term, catalog dataset, or concept assetGET /plane/catalog-objectsandGET /plane/catalog-objects/{id}— browse the tenant planeGET /plane/query?q=...— find objects by title, alias, concept key, or document-KG identifierPOST .../document-kg-links— attach an opaque naruon/DiskSage/commons reference without ingesting contentPOST .../concept-bindings— attach an ontology concept keyEvery successful response includes Korean buyer guidance in
customer_next_actionand the recordedpolicy_decision_id. Steward display names remain usable under purpose-bound authorization rather than indiscriminate masking.Identity, policy, and trust boundary
Production requests use:
Authorization: Bearer <Keyverse access token>X-CWL-Tenant-ReferenceX-CWL-Access-Purpose(catalog_browse|glossary_stewardship|ontology_query|document_kg_alignment)Raw
X-CWL-Oidc-Subjectauthentication is disabled by default, including when JWKS coordinates are absent. A non-external local demo or CI environment may explicitly setSDP_ALLOW_UNVERIFIED_SUBJECT_HEADER=true; JWKS-configured environments reject the raw header even when that flag is present. Paid-pilot and production deployments must not enable the demo flag.All
/plane/*operations invoke the existingpolicy.evaluate()before catalog access. The existing policy evidence store records allow/deny decisions; successful envelopes return the corresponding decision ID. SDP does not provision tenants, an IdP, or SCIM.Persistence and data model
SDP_DATABASE_DSNis unset.SDP_DATABASE_DSN, create/list/get/query/attach use migration0002so paid-pilot restarts retain catalog rows.snake_case:catalog_objects,object_definitions,object_aliases,document_kg_links,concept_object_bindings,commons_score_references,object_stewards.Boundaries preserved
Review remediation
The initial CodeRabbit review was against
aa53a8eand became stale after the following exact-source repairs:RLockaround the in-memory store's check/write/iteration boundaryLiteral-derived runtime value sets and typedobject_kindquery validationResolved review threads were closed only after checking the current source. The raw subject-header finding is addressed by the new default-deny regression and implementation; it remains to be resolved after exact-head CI confirms the change.
TDD and verification
The default-deny subject-header regression was added before the production change. The focused RED reproduced the vulnerable behavior as
Failed: DID NOT RAISE TenantBindingError; the minimal implementation then made both the default-deny case and explicit-demo-opt-in case pass in the focused harness.Current exact head:
bfa409f8971a3b28aac31ec7bca371fa94f7f07f.Exact-head repository Tests, fuzz, Security Scan, and SAST runs were triggered by this source change and remain authoritative. Do not merge from predecessor-head evidence. Merge still requires every live required check, all review threads resolved, and a qualifying independent current-head approval.
Summary by CodeRabbit
새로운 기능
보안
문서
개선