Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/domain/semantic-layer/specs/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,10 @@ Definitions live in application-owned storage so they can be authored, versioned

Every dataset publishes a typed, role-annotated catalog of its fields — the editor's palette, the compiler's validation universe, and the discovery API's vocabulary at once. It is what makes capability a projection of definitions rather than data.

**Implementation status (#2208, Phase 1 slice 2):** the build-time catalog is shipped in `domain/field_catalog/` (Rust). Because the dbt `schema.yml` files carry no `meta:` type/role annotations, the source is **hybrid**: column **types** come from a committed ClickHouse type snapshot (`types.snapshot.json`, generated from `system.columns` and kept honest by an ignored drift test against the live warehouse), and **roles** are authored in `roles.yaml`. The generator joins the two and rejects any role that names an absent column or an unmodeled type, so the embedded catalog is a consistent validation universe at build time (offline tests) with no live warehouse needed in CI. Inspect via `analytics field-catalog`.

Coverage is the source families with a clean, self-contained entity and tenant at silver — **git, ai, collab, wiki** (12 datasets). Deferred to the extraction slice (slice 5), where roles are verified against real measures: **task** (`class_task_worklogs`/`comments` carry no tenant column and key on `author_id`, not an email — needs a tenant join and identity resolution), the reference/join relations with no direct entity (repository/branch, git PR comment/commit/reviewer, task statuses/projects/sprints/field-metadata, wiki engagement), and derived populations (active-day, focus). Two cross-family facts the catalog already absorbs: the tenant column name is **not** uniform (`tenant_id` for git/collab/wiki, `insight_tenant_id` for ai), named per dataset in `roles.yaml`; and `Decimal(p,s)` amount columns are a first-class field type distinct from `Float`.
Comment on lines +285 to +287

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Align the catalog source and construction model.

This text describes a build-time generator. The implementation constructs the embedded catalog lazily through OnceLock::get_or_init. It also conflicts with Line 291, which still says dbt schema.yml metadata is the source and roles are never hand-maintained. Update this section and the responsibility scope so they describe the same hybrid roles.yaml and type-snapshot model.

🤖 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 285 - 287, Update
the catalog description and responsibility scope to match the implementation’s
lazy OnceLock::get_or_init construction, using roles.yaml for authored roles and
types.snapshot.json for column types. Remove or revise the conflicting claim
that dbt schema.yml metadata is the source and roles are never hand-maintained,
while preserving the documented dataset coverage and deferred responsibilities.


##### Responsibility scope

- Generated from dataset schemas (a Rust build-time generator sharing the backend's definition parsers, sourced from dbt `schema.yml` `meta:` blocks), never hand-maintained. Roles: entity, dimension, measurable, event time.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
//! Drift test: the committed type snapshot (via the built catalog) must still
//! agree with the live ClickHouse schema. For every catalogued field, the
//! warehouse's column type — normalized — must equal the field's type in the
//! catalog. A mismatch means `types.snapshot.json` is stale and must be
//! regenerated from ClickHouse.
//!
//! Ignored by default; requires a live ClickHouse. Enable with
//! `INTEGRATION_TESTS_CLICKHOUSE_URL=<url> cargo test -p analytics -- --ignored
//! field_catalog`.

use std::collections::BTreeMap;

use clickhouse::Row;
use serde::Deserialize;

use crate::domain::field_catalog::field_catalog;
use crate::domain::field_catalog::model::FieldType;

const CH_ENV: &str = "INTEGRATION_TESTS_CLICKHOUSE_URL";
const CH_USER_ENV: &str = "INTEGRATION_TESTS_CLICKHOUSE_USER";
const CH_PASSWORD_ENV: &str = "INTEGRATION_TESTS_CLICKHOUSE_PASSWORD";

#[derive(Debug, Row, Deserialize)]
struct ColumnRow {
name: String,
#[serde(rename = "type")]
ch_type: String,
}

#[tokio::test]
#[ignore = "requires live ClickHouse; set INTEGRATION_TESTS_CLICKHOUSE_URL to enable"]
async fn types_snapshot_matches_clickhouse() {
let Ok(url) = std::env::var(CH_ENV) else {
eprintln!("skipping: {CH_ENV} not set");
return;
};
let mut config = insight_clickhouse::Config::new(&url, "default");
if let (Ok(user), Ok(password)) = (std::env::var(CH_USER_ENV), std::env::var(CH_PASSWORD_ENV)) {
config = config.with_auth(user, password);
}
let client = insight_clickhouse::Client::new(config);

let mut drift: Vec<String> = Vec::new();

for dataset in &field_catalog().datasets {
let rows = client
.query("SELECT name, type FROM system.columns WHERE database = ? AND table = ?")
.bind(dataset.database.as_str())
.bind(dataset.table.as_str())
.fetch_all::<ColumnRow>()
.await
.unwrap_or_else(|e| panic!("querying columns of {}: {e}", dataset.relation()));

let live: BTreeMap<String, String> =
rows.into_iter().map(|r| (r.name, r.ch_type)).collect();
Comment on lines +46 to +55

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate file =="
git ls-files | rg 'src/backend/services/analytics/src/domain/field_catalog/live_tests\.rs$|clickhouse|ColumnRow' || true

echo "== target file outline/contents =="
if [ -f src/backend/services/analytics/src/domain/field_catalog/live_tests.rs ]; then
  wc -l src/backend/services/analytics/src/domain/field_catalog/live_tests.rs
  sed -n '1,140p' src/backend/services/analytics/src/domain/field_catalog/live_tests.rs | nl -ba
fi

echo "== all system.columns query usages =="
rg -n 'system\.columns|fetch_all::<ColumnRow>|fetch_all\(' src/backend/services/analytics || true

echo "== lock/clickhouse deps mentions =="
rg -n 'clickhouse|Client|fetch_all|limit|LIMIT' src/backend/services/analytics/Cargo.toml src/backend/services/analytics -g '*.rs' -g 'Cargo.toml' || true

Repository: constructorfabric/insight

Length of output: 1500


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== exact target file contents with line numbers =="
sed -n '1,220p' src/backend/services/analytics/src/domain/field_catalog/live_tests.rs | nl -ba

echo "== lock files for package version context, if present =="
git ls-files '*lock*' 'Cargo.lock' | sed -n '1,40p'
if [ -f Cargo.lock ]; then
  python3 - <<'PY'
import re
p='Cargo.lock'
txt=open(p,errors='ignore').read().split('[[package]]\n')[1:]
for blk in txt:
    if 'clickhouse' in blk.lower() or 'click' in blk.lower():
        m=re.search(r'name\s*=\s*"([^"]*)"\nversion\s*=\s*"([^"]*)"', blk)
        print(m.group(0).replace('\n',' | ') if m else blk[:500])
PY
fi

Repository: constructorfabric/insight

Length of output: 254


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== line count =="
wc -l src/backend/services/analytics/src/domain/field_catalog/live_tests.rs
wc -l src/backend/libs/insight-clickhouse/src/query.rs

echo "== target file contents =="
sed -n '1,130p' src/backend/services/analytics/src/domain/field_catalog/live_tests.rs

echo "== query.rs contents =="
sed -n '1,180p' src/backend/libs/insight-clickhouse/src/query.rs

echo "== all system.columns query usage context =="
rg -n -C 4 'system\.columns|fetch_all::<ColumnRow>|fetch_all\(' src/backend/services/analytics src/backend/libs/insight-clickhouse || true

Repository: constructorfabric/insight

Length of output: 25872


Bound the system.columns response in the live-schema test.

This test calls fetch_all::<ColumnRow>() on an unbounded SELECT name, type FROM system.columns WHERE database = ? AND table = ?, so a relation with too many columns can exhaust test memory. Add a response-size limit in the query/streamed read, or fetch N + 1 rows and report a clear over-limit failure instead of panicking.

🤖 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/backend/services/analytics/src/domain/field_catalog/live_tests.rs` around
lines 46 - 55, Bound the `system.columns` read in the live-schema test around
the `fetch_all::<ColumnRow>()` call, using the project’s established query limit
or streamed-read mechanism. Ensure relations exceeding the limit produce a clear
over-limit failure while preserving the existing `BTreeMap` construction for
valid responses.

Source: Coding guidelines


if live.is_empty() {
drift.push(format!(
"{}: relation absent from ClickHouse",
dataset.relation()
));
continue;
}

for field in &dataset.fields {
match live.get(&field.name) {
None => drift.push(format!(
"{}.{}: column absent from ClickHouse",
dataset.relation(),
field.name
)),
Some(ch_type) => match FieldType::normalize(ch_type) {
Some((ty, nullable)) if ty == field.ty && nullable == field.nullable => {}
other => drift.push(format!(
"{}.{}: catalog has {:?} (nullable={}), ClickHouse has {ch_type} -> {other:?}",
dataset.relation(),
field.name,
field.ty,
field.nullable
)),
},
}
}
}

assert!(
drift.is_empty(),
"field-catalog type snapshot is stale — regenerate types.snapshot.json from ClickHouse:\n{}",
drift.join("\n")
);
}
Loading