docs(semantic-layer): adopt target architecture for Phase B - #2184
Merged
cyberantonz merged 4 commits intoAug 4, 2026
Merged
Conversation
Adopt the semantic-layer design (definitions-as-data, one compiler over datasets, custom-dataset SQL escape hatch) as the governing Phase B design for the presentation split, under docs/domain/semantic-layer/specs/. - DESIGN.md / IMPLEMENTATION.md: the target architecture and its phased, parity-checked migration, adopted verbatim from the research. - FINDINGS.md: adoption review — how the shipped YAML registry is this design's Phase 1 first step; the sub-issue re-scope (compiler-first drops the generated-SQL drift gate); and the org-scope authorization the design must name (Security: no people outside your org scope; Scope isolation: no other teams/cohorts), injected server-side beside tenancy. Governed in-repo following the metrics-domain precedent; conversion into the strict cfs sdlc DESIGN template (+ companion PRD) is deferred to avoid distorting the design before the schema-rewrite decision is committed. Part of constructorfabric#1803 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
📝 WalkthroughWalkthroughAdds governed documentation for a compiler-first semantic layer. It defines semantic contracts, execution and storage rules, migration phases, discovery and editing APIs, authorization requirements, and architecture decisions. ChangesSemantic layer architecture
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Scope isolation must hold across cohort membership: an R&D member and a Sales member who share a tag are never in the same cohort. The org chart gates cohort composition; tags refine within the boundary, never across it. Notes the current gap (only org_unit cohorts are implicitly org-scoped; arbitrary/tag cohort keys are not yet intersected with org visibility) and where it must be enforced (cohort dataset + compiler injected scope, constructorfabric#1980). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@docs/domain/semantic-layer/specs/DESIGN.md`:
- Around line 48-54: Add the text language tag to both diagram code fences:
docs/domain/semantic-layer/specs/DESIGN.md lines 48-54 and
docs/domain/semantic-layer/specs/IMPLEMENTATION.md lines 295-299. Keep the
diagram contents unchanged.
- Around line 429-434: Update the per-input read decision described in the
cache-serving flow to check definition availability before selecting cached rows
or compiling live data. When availability is not available, return the stored
unavailable error; only allow cached or live execution when availability is
available, while preserving the existing policy, version, and coverage checks.
- Around line 418-424: Clarify the custom-dataset rebuild semantics in the
DESIGN.md section around the definition DAG refresh: permit serving the previous
table only when refresh fails without a semantic definition-version change.
After a version bump, invalidate the superseded table and require reads to use
the live view or return unavailable; do not allow failed rebuilds to serve
superseded custom-dataset data.
- Around line 162-175: Align percentile support across all three documented
sites: in docs/domain/semantic-layer/specs/DESIGN.md lines 162-175, add the
complete percentile aggregation contract to the measure schema; in
docs/domain/semantic-layer/specs/DESIGN.md lines 230-235, update the
expressiveness claim to include percentile; and in
docs/domain/semantic-layer/specs/IMPLEMENTATION.md lines 119-124, document the
required validator, compiler, and storage work for percentile.
In `@docs/domain/semantic-layer/specs/FINDINGS.md`:
- Around line 81-84: Update the stale DESIGN.md reference in FINDINGS.md to
point to the document’s actual unnumbered Principles heading, or revise
DESIGN.md to add the referenced “§2 (Principles & Constraints)” structure;
ensure the Compiler section reference also matches an existing section.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b08f5d60-b115-4568-b9df-577957390f82
📒 Files selected for processing (4)
docs/domain/semantic-layer/specs/DESIGN.mddocs/domain/semantic-layer/specs/FINDINGS.mddocs/domain/semantic-layer/specs/IMPLEMENTATION.mddocs/domain/semantic-layer/specs/README.md
Author template-conformant PRD.md and DESIGN.md for the semantic layer and register them as cfs sdlc artifacts (new `semantic-layer` system in artifacts.toml). Both pass per-artifact cfs validation (toc + validate + check-language) and add zero new whole-registry errors. - PRD.md / DESIGN.md: the governed artifacts (cpt-semantic-layer-* IDs), distilled from the reference narrative; injected scopes (tenant, org-scope entity visibility, cohort isolation) are first-class FRs/NFRs, a principle, a component, and a sequence. - REFERENCE.md: the original design narrative, kept verbatim as the deep rationale the governed specs cite. - FINDINGS.md: adds the three open review items (percentile capability; no serving a superseded custom-dataset after a version bump; gate cache reads on availability) and points at the governed specs. - Fold in CodeRabbit doc fixes: language-tag the diagram fences; fix the stale section reference. Part of constructorfabric#1803 Signed-off-by: Anton Zelenov <antonz@constructor.tech>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@docs/domain/semantic-layer/specs/DESIGN.md`:
- Around line 678-688: Update the dataset schema around
datasets.database_relation to model warehouse relations and custom SQL as
distinct source variants, adding a source discriminator with separate
relation_name and validated select_sql fields. Move source-specific availability
and captured role-annotated schema into the selected variant, and update the
surrounding contract descriptions to reflect the distinct validation, quoting,
lineage, and execution paths.
- Around line 753-770: The measure_cache schema must store mergeable state
rather than a single numeric value for non-additive measures. Update the measure
cache design around measure_cache and its PK/additional-info rules to define
aggregation-specific state shapes and merge rules, including sum/count
components for avg and subject identity or equivalent mergeable distinct state
for count_distinct, before permitting those aggregations to be cached.
- Around line 753-770: Update the measure_cache schema and related semantics to
include insight_tenant_id UUID in each row, the primary key, partitioning,
refresh/invalidation scope, and cache-read predicates so cached work is
tenant-scoped. Extend the cache identity to include reporting timezone, or
explicitly define and use a canonical UTC representation consistently.
In `@docs/domain/semantic-layer/specs/PRD.md`:
- Around line 271-275: The Materialization Cache requirement currently permits
live computation for unavailable definitions. Update the PRD requirement and its
corresponding DESIGN.md rule to return the stored unavailable-definition error
without cached or live rows, while retaining live fallback only for stale
versions, uncovered ranges, and disabled policy.
- Around line 185-190: Update the custom-dataset deletion requirement in the
gated SQL layer specification to block deletion whenever any definition
references the dataset, including direct references from dependent custom
datasets, not only measure references. Align the wording with the
dependency-protection behavior specified by DESIGN.md.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7b662fa2-f4b1-4fae-a7c3-d7ded549e0d3
📒 Files selected for processing (7)
.cf-studio/config/artifacts.tomldocs/domain/semantic-layer/specs/DESIGN.mddocs/domain/semantic-layer/specs/FINDINGS.mddocs/domain/semantic-layer/specs/IMPLEMENTATION.mddocs/domain/semantic-layer/specs/PRD.mddocs/domain/semantic-layer/specs/README.mddocs/domain/semantic-layer/specs/REFERENCE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/domain/semantic-layer/specs/IMPLEMENTATION.md
Comment on lines
+678
to
+688
| | Column | Type | Description | | ||
| |--------|------|-------------| | ||
| | `key` | String | Stable dataset identifier | | ||
| | `database_relation` | String | Warehouse database + relation (or the registered custom SELECT) | | ||
| | `read_discipline` | Enum | Dedup strategy the compiler inherits | | ||
| | `retention_horizon` | Interval | History depth part of the served contract | | ||
| | `origin` | Enum | `product` \| `custom` | | ||
|
|
||
| **PK**: `key` | ||
|
|
||
| **Additional info**: Custom-dataset rows carry the validated SELECT and its captured, role-annotated schema; availability state is stored on the row and read by serving/discovery. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Separate relation identifiers from custom SQL bodies.
datasets.database_relation is defined as either a warehouse relation or a registered custom SELECT. These values require different validation, quoting, lineage, and execution paths.
Add a source discriminator and separate fields such as relation_name and validated select_sql. Keep availability and captured schema attached to the selected source variant.
🤖 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 `@docs/domain/semantic-layer/specs/DESIGN.md` around lines 678 - 688, Update
the dataset schema around datasets.database_relation to model warehouse
relations and custom SQL as distinct source variants, adding a source
discriminator with separate relation_name and validated select_sql fields. Move
source-specific availability and captured role-annotated schema into the
selected variant, and update the surrounding contract descriptions to reflect
the distinct validation, quoting, lineage, and execution paths.
cyberantonz
enabled auto-merge
August 4, 2026 11:55
aleksdotbar
approved these changes
Aug 4, 2026
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
2 participants
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.
Adopts the semantic-layer design as the governing Phase B architecture for the presentation-layer split. Part of #1803.
What
Brings the design into the repo under
docs/domain/semantic-layer/specs/:builtin.rs); its observation-relation schema is transitional and gets rewritten at cutover.person_visibility→ identity/v1/visible-persons, moving into the compiler's injected scope beside tenancy ([pres] Tenant hierarchy/subtree + row-policy backstop #1980).Governance
Adopted as a governed in-repo doc following the metrics-domain precedent (
docs/domain/metrics/specs/DESIGN.md, itself governing-but-unregistered). Converting it into the strictcfssdlc DESIGN template (numbered sections,cpt-semantic-*IDs, Functional-Driver/NFR tables, a companion PRD) is a larger, separate task — deferred so the reformat does not distort the design before the schema-rewrite decision is committed.🤖 Generated with Claude Code
Summary by CodeRabbit