test(e2e): Wiki bullet metrics — Outline + Confluence (#1440) - #1510
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughAdds Bronze placeholder databases and wiki tables for Outline and Confluence, updates the Jira user placeholder contract with ChangesWiki bronze and e2e fixtures
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ingestion/scripts/create-bronze-placeholders.sh (1)
736-758: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReconcile existing
jira_userplaceholders when addingtenant_id.Line 742 fixes fresh tables, but warm ClickHouse/e2e clusters with an older
bronze_jira.jira_userskip this branch and keep the old schema.confluence__wiki_pagesreadstenant_id, so reruns can still fail unless the existing table is altered.Proposed fix
if ! ch_table_exists bronze_jira jira_user; then echo " Creating placeholder: bronze_jira.jira_user" run_ch <<'SQL' CREATE TABLE IF NOT EXISTS bronze_jira.jira_user ( @@ _airbyte_generation_id UInt32 DEFAULT 0 ) ENGINE = ReplacingMergeTree(_airbyte_extracted_at) ORDER BY unique_key; SQL +else + echo " Reconciling placeholder schema: bronze_jira.jira_user" + run_ch <<'SQL' +ALTER TABLE bronze_jira.jira_user ADD COLUMN IF NOT EXISTS tenant_id Nullable(String); +SQL fi🤖 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/ingestion/scripts/create-bronze-placeholders.sh` around lines 736 - 758, The jira_user placeholder creation only handles new tables, so existing bronze_jira.jira_user tables can remain on the old schema without tenant_id. Update the create-bronze-placeholders.sh logic around the jira_user block to reconcile pre-existing tables by adding an ALTER TABLE path (or equivalent schema migration) when ch_table_exists bronze_jira jira_user is true but tenant_id is missing, ensuring confluence__wiki_pages can always read tenant_id. Use the bronze_jira.jira_user creation block and its ch_table_exists check as the place to detect and upgrade older schemas.
🤖 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
`@src/ingestion/tests/e2e/metrics/schemas/bronze_confluence.wiki_footer_comments.yaml`:
- Around line 7-20: Add the new parent_comment_id field to the strict Confluence
footer-comments schema so fixtures with that placeholder validate successfully
under additionalProperties: false. Update
bronze_confluence.wiki_footer_comments.yaml alongside the existing comment
fields (such as page_id, comment_id, and author_id) to include parent_comment_id
with the same nullable string shape used by the other optional IDs.
In `@src/ingestion/tests/e2e/metrics/schemas/bronze_outline.wiki_comments.yaml`:
- Around line 4-22: The bronze_outline.wiki_comments schema currently only
blocks unknown fields via additionalProperties, but still allows required
columns to be omitted. Update the wiki_comments schema definition to add a
required list covering the expected bronze contract fields, using the existing
property names in bronze_outline.wiki_comments so fixtures must include them
during validation. Keep additionalProperties: false, and make sure the required
set matches the columns that must always be present (including identifiers and
timestamps such as page_id, comment_id, author_id, and created_at).
In `@src/ingestion/tests/e2e/metrics/schemas/bronze_outline.wiki_pages.yaml`:
- Around line 4-28: The bronze_outline.wiki_pages schema is missing a required
list, so it still accepts rows that omit expected fields while blocking only
unknown ones. Update the schema definition in bronze_outline.wiki_pages.yaml to
add a required section for the mandatory columns used by this contract,
especially the core identifiers and timestamps such as page_id, author_id,
version_number, and created_at. Keep the existing properties and
additionalProperties setting unchanged, and ensure the required set matches the
strict bronze expectations for this fixture.
---
Outside diff comments:
In `@src/ingestion/scripts/create-bronze-placeholders.sh`:
- Around line 736-758: The jira_user placeholder creation only handles new
tables, so existing bronze_jira.jira_user tables can remain on the old schema
without tenant_id. Update the create-bronze-placeholders.sh logic around the
jira_user block to reconcile pre-existing tables by adding an ALTER TABLE path
(or equivalent schema migration) when ch_table_exists bronze_jira jira_user is
true but tenant_id is missing, ensuring confluence__wiki_pages can always read
tenant_id. Use the bronze_jira.jira_user creation block and its ch_table_exists
check as the place to detect and upgrade older schemas.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35b3775a-03d4-483d-8467-e97f09549e1a
📥 Commits
Reviewing files that changed from the base of the PR and between 95009a5 and 09016b085fae40ae0c47fd0b1defb6e70cbfafe4.
📒 Files selected for processing (18)
src/ingestion/scripts/create-bronze-placeholders.shsrc/ingestion/tests/e2e/conftest.pysrc/ingestion/tests/e2e/metrics/schemas/bronze_confluence.wiki_footer_comments.yamlsrc/ingestion/tests/e2e/metrics/schemas/bronze_confluence.wiki_pages.yamlsrc/ingestion/tests/e2e/metrics/schemas/bronze_outline.wiki_comments.yamlsrc/ingestion/tests/e2e/metrics/schemas/bronze_outline.wiki_pages.yamlsrc/ingestion/tests/e2e/metrics/templates/confluence_wiki_footer_comments.yamlsrc/ingestion/tests/e2e/metrics/templates/confluence_wiki_pages.yamlsrc/ingestion/tests/e2e/metrics/templates/outline_wiki_comments.yamlsrc/ingestion/tests/e2e/metrics/templates/outline_wiki_pages.yamlsrc/ingestion/tests/e2e/metrics/wiki_confluence_active_authors.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_confluence_comments.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_confluence_edits.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_confluence_pages_created.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_outline_active_authors.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_outline_comments.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_outline_edits.test.yamlsrc/ingestion/tests/e2e/metrics/wiki_outline_pages_created.test.yaml
| bronze_outline.wiki_comments: | ||
| $schema: http://json-schema.org/draft-07/schema# | ||
| type: object | ||
| additionalProperties: false | ||
| properties: | ||
| _airbyte_raw_id: { type: string } | ||
| _airbyte_extracted_at: { type: string, format: date-time } | ||
| _airbyte_meta: { type: string } | ||
| _airbyte_generation_id: { type: integer } | ||
| unique_key: { type: string } | ||
| tenant_id: { type: [string, "null"] } | ||
| source_id: { type: [string, "null"] } | ||
| page_id: { type: [string, "null"] } | ||
| comment_id: { type: [string, "null"] } | ||
| author_id: { type: [string, "null"] } | ||
| created_at: { type: [string, "null"] } | ||
| resolution_status: { type: [string, "null"] } | ||
| parent_comment_id: { type: [string, "null"] } | ||
| anchor_text: { type: [string, "null"] } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make the schema enforce column presence, not just reject extras.
additionalProperties: false only blocks unknown keys; without a required list, a fixture can still omit page_id, comment_id, author_id, created_at, etc. and pass validation. That weakens this file’s role as the strict bronze contract and can let broken placeholders slip through until the dbt model runs.
Proposed fix
schemas:
bronze_outline.wiki_comments:
$schema: http://json-schema.org/draft-07/schema#
type: object
additionalProperties: false
+ required:
+ - _airbyte_raw_id
+ - _airbyte_extracted_at
+ - _airbyte_meta
+ - _airbyte_generation_id
+ - unique_key
+ - tenant_id
+ - source_id
+ - page_id
+ - comment_id
+ - author_id
+ - created_at
+ - resolution_status
+ - parent_comment_id
+ - anchor_text
properties:
_airbyte_raw_id: { type: string }
_airbyte_extracted_at: { type: string, format: date-time }
_airbyte_meta: { type: string }
_airbyte_generation_id: { type: integer }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bronze_outline.wiki_comments: | |
| $schema: http://json-schema.org/draft-07/schema# | |
| type: object | |
| additionalProperties: false | |
| properties: | |
| _airbyte_raw_id: { type: string } | |
| _airbyte_extracted_at: { type: string, format: date-time } | |
| _airbyte_meta: { type: string } | |
| _airbyte_generation_id: { type: integer } | |
| unique_key: { type: string } | |
| tenant_id: { type: [string, "null"] } | |
| source_id: { type: [string, "null"] } | |
| page_id: { type: [string, "null"] } | |
| comment_id: { type: [string, "null"] } | |
| author_id: { type: [string, "null"] } | |
| created_at: { type: [string, "null"] } | |
| resolution_status: { type: [string, "null"] } | |
| parent_comment_id: { type: [string, "null"] } | |
| anchor_text: { type: [string, "null"] } | |
| bronze_outline.wiki_comments: | |
| $schema: http://json-schema.org/draft-07/schema# | |
| type: object | |
| additionalProperties: false | |
| required: | |
| - _airbyte_raw_id | |
| - _airbyte_extracted_at | |
| - _airbyte_meta | |
| - _airbyte_generation_id | |
| - unique_key | |
| - tenant_id | |
| - source_id | |
| - page_id | |
| - comment_id | |
| - author_id | |
| - created_at | |
| - resolution_status | |
| - parent_comment_id | |
| - anchor_text | |
| properties: | |
| _airbyte_raw_id: { type: string } | |
| _airbyte_extracted_at: { type: string, format: date-time } | |
| _airbyte_meta: { type: string } | |
| _airbyte_generation_id: { type: integer } | |
| unique_key: { type: string } | |
| tenant_id: { type: [string, "null"] } | |
| source_id: { type: [string, "null"] } | |
| page_id: { type: [string, "null"] } | |
| comment_id: { type: [string, "null"] } | |
| author_id: { type: [string, "null"] } | |
| created_at: { type: [string, "null"] } | |
| resolution_status: { type: [string, "null"] } | |
| parent_comment_id: { type: [string, "null"] } | |
| anchor_text: { type: [string, "null"] } |
🤖 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/ingestion/tests/e2e/metrics/schemas/bronze_outline.wiki_comments.yaml`
around lines 4 - 22, The bronze_outline.wiki_comments schema currently only
blocks unknown fields via additionalProperties, but still allows required
columns to be omitted. Update the wiki_comments schema definition to add a
required list covering the expected bronze contract fields, using the existing
property names in bronze_outline.wiki_comments so fixtures must include them
during validation. Keep additionalProperties: false, and make sure the required
set matches the columns that must always be present (including identifiers and
timestamps such as page_id, comment_id, author_id, and created_at).
| bronze_outline.wiki_pages: | ||
| $schema: http://json-schema.org/draft-07/schema# | ||
| type: object | ||
| additionalProperties: false | ||
| properties: | ||
| _airbyte_raw_id: { type: string } | ||
| _airbyte_extracted_at: { type: string, format: date-time } | ||
| _airbyte_meta: { type: string } | ||
| _airbyte_generation_id: { type: integer } | ||
| unique_key: { type: string } | ||
| tenant_id: { type: [string, "null"] } | ||
| source_id: { type: [string, "null"] } | ||
| page_id: { type: [string, "null"] } | ||
| space_id: { type: [string, "null"] } | ||
| title: { type: [string, "null"] } | ||
| status: { type: [string, "null"] } | ||
| author_id: { type: [string, "null"] } | ||
| author_email: { type: [string, "null"] } | ||
| last_editor_id: { type: [string, "null"] } | ||
| last_editor_email: { type: [string, "null"] } | ||
| parent_page_id: { type: [string, "null"] } | ||
| version_number: { type: [number, "null"] } | ||
| created_at: { type: [string, "null"] } | ||
| updated_at: { type: [string, "null"] } | ||
| collected_at: { type: [string, "null"] } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
This schema has the same missing-required gap.
Right now it rejects unexpected fields but still accepts rows that omit expected ones like page_id, author_id, version_number, or created_at. For a strict bronze contract, that means fixture/schema drift can pass validation and only fail later in downstream models.
Proposed fix
schemas:
bronze_outline.wiki_pages:
$schema: http://json-schema.org/draft-07/schema#
type: object
additionalProperties: false
+ required:
+ - _airbyte_raw_id
+ - _airbyte_extracted_at
+ - _airbyte_meta
+ - _airbyte_generation_id
+ - unique_key
+ - tenant_id
+ - source_id
+ - page_id
+ - space_id
+ - title
+ - status
+ - author_id
+ - author_email
+ - last_editor_id
+ - last_editor_email
+ - parent_page_id
+ - version_number
+ - created_at
+ - updated_at
+ - collected_at
properties:
_airbyte_raw_id: { type: string }
_airbyte_extracted_at: { type: string, format: date-time }
_airbyte_meta: { type: string }
_airbyte_generation_id: { type: integer }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bronze_outline.wiki_pages: | |
| $schema: http://json-schema.org/draft-07/schema# | |
| type: object | |
| additionalProperties: false | |
| properties: | |
| _airbyte_raw_id: { type: string } | |
| _airbyte_extracted_at: { type: string, format: date-time } | |
| _airbyte_meta: { type: string } | |
| _airbyte_generation_id: { type: integer } | |
| unique_key: { type: string } | |
| tenant_id: { type: [string, "null"] } | |
| source_id: { type: [string, "null"] } | |
| page_id: { type: [string, "null"] } | |
| space_id: { type: [string, "null"] } | |
| title: { type: [string, "null"] } | |
| status: { type: [string, "null"] } | |
| author_id: { type: [string, "null"] } | |
| author_email: { type: [string, "null"] } | |
| last_editor_id: { type: [string, "null"] } | |
| last_editor_email: { type: [string, "null"] } | |
| parent_page_id: { type: [string, "null"] } | |
| version_number: { type: [number, "null"] } | |
| created_at: { type: [string, "null"] } | |
| updated_at: { type: [string, "null"] } | |
| collected_at: { type: [string, "null"] } | |
| bronze_outline.wiki_pages: | |
| $schema: http://json-schema.org/draft-07/schema# | |
| type: object | |
| additionalProperties: false | |
| required: | |
| - _airbyte_raw_id | |
| - _airbyte_extracted_at | |
| - _airbyte_meta | |
| - _airbyte_generation_id | |
| - unique_key | |
| - tenant_id | |
| - source_id | |
| - page_id | |
| - space_id | |
| - title | |
| - status | |
| - author_id | |
| - author_email | |
| - last_editor_id | |
| - last_editor_email | |
| - parent_page_id | |
| - version_number | |
| - created_at | |
| - updated_at | |
| - collected_at | |
| properties: | |
| _airbyte_raw_id: { type: string } | |
| _airbyte_extracted_at: { type: string, format: date-time } | |
| _airbyte_meta: { type: string } | |
| _airbyte_generation_id: { type: integer } | |
| unique_key: { type: string } | |
| tenant_id: { type: [string, "null"] } | |
| source_id: { type: [string, "null"] } | |
| page_id: { type: [string, "null"] } | |
| space_id: { type: [string, "null"] } | |
| title: { type: [string, "null"] } | |
| status: { type: [string, "null"] } | |
| author_id: { type: [string, "null"] } | |
| author_email: { type: [string, "null"] } | |
| last_editor_id: { type: [string, "null"] } | |
| last_editor_email: { type: [string, "null"] } | |
| parent_page_id: { type: [string, "null"] } | |
| version_number: { type: [number, "null"] } | |
| created_at: { type: [string, "null"] } | |
| updated_at: { type: [string, "null"] } | |
| collected_at: { type: [string, "null"] } |
🤖 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/ingestion/tests/e2e/metrics/schemas/bronze_outline.wiki_pages.yaml`
around lines 4 - 28, The bronze_outline.wiki_pages schema is missing a required
list, so it still accepts rows that omit expected fields while blocking only
unknown ones. Update the schema definition in bronze_outline.wiki_pages.yaml to
add a required section for the mandatory columns used by this contract,
especially the core identifiers and timestamps such as page_id, author_id,
version_number, and created_at. Keep the existing properties and
additionalProperties setting unchanged, and ensure the required set matches the
strict bronze expectations for this fixture.
|
Thanks @coderabbitai — addressed in f549e2ed:
On the Re-verified: |
|
Tip For best results, initiate chat on the files or code changes.
Your rationale on omitting Also great to see the ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
E2E specs for all 4 wiki metrics on IC Bullet Wiki (…0041), one spec per
(connector, metric) — 8 total:
- wiki_pages_created (sum, 1/page)
- wiki_edits (sum, Σ version_count − 1)
- wiki_active_authors (active marker; value 1, median 0, range_max = headcount)
- wiki_comments (sum, comments attributed to the page author)
Pipeline: bronze_{outline,confluence}.* → {outline,confluence}__wiki_{pages,
engagement} → silver class_wiki_pages / class_wiki_engagement (union_by_tag) →
gold insight.wiki_bullet_rows → IC Bullet Wiki (…0041). Both connectors feed the
SAME silver union, so both connectors' bronze must exist for it to run — this PR
adds placeholders for both even though each spec seeds only its own.
Supporting:
- bronze placeholders: bronze_outline.{wiki_pages,wiki_spaces,wiki_users,
wiki_comments} + bronze_confluence.{wiki_pages,wiki_spaces,wiki_footer_comments,
wiki_footer_comment_replies,wiki_inline_comments,wiki_inline_comment_replies}
(+ schemas/templates for the seeded ones).
- bronze_jira.jira_user placeholder: add the missing `tenant_id` column —
confluence__wiki_pages' jira_user identity join reads it, and without it the
model fails to compile on a fresh cluster (latent placeholder gap).
- confluence comment placeholders carry parent_comment_id (the reply-integrity
singular tests in tests/wiki/ reference it).
- conftest: reset class_wiki_pages / class_wiki_engagement / class_wiki_activity
at session start (incremental delete+insert unions both connectors → warm-rerun
determinism). Confluence runs standalone (no Jira) → person_id falls back to
author_id; specs set author_id to the email so identity resolves to a seeded
person.
Verified locally: 8 passed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…id schema - create-bronze-placeholders.sh: add an `else` ALTER branch so a pre-existing bronze_jira.jira_user (warm cluster / created earlier by the Jira connector) gets `tenant_id` added in place — the `if ! ch_table_exists` create branch is skipped for it, and confluence__wiki_pages' jira_user join needs the column. Idempotent via ADD COLUMN IF NOT EXISTS. - bronze_confluence.wiki_footer_comments schema + template: list parent_comment_id (the placeholder carries it; additionalProperties:false would otherwise reject a fixture that sets it). Left as-is: no `required:` lists — none of the 16 existing e2e schemas use them (the convention is additionalProperties:false + base templates that carry every column), so adding them only here would be inconsistent. 8 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
0b75994 to
269dabe
Compare
What
E2E specs for the Wiki bullet (IC Bullet Wiki,
…0041) across both wiki-class connectors — Outline and Confluence. One spec per (connector, metric), 8 total:wiki_pages_createdclass_wiki_pageswiki_editsclass_wiki_pageswiki_active_authorsclass_wiki_pageswiki_commentsclass_wiki_engagement⋈ pagesPipeline:
bronze_{outline,confluence}.*→{outline,confluence}__wiki_{pages,engagement}→ silverclass_wiki_pages/class_wiki_engagement(union_by_tag) → goldinsight.wiki_bullet_rows→ IC Bullet Wiki.Values (per connector): pages 3·2·1 → median 2, range [1,3]; edits v5/v3/v2 → 4·2·1, median 2, range [1,4]; active 1-of-3; comments 3·2·1 on alice/bob/carol's pages → median 2, range [1,3]. Each seeds a re-sync duplicate to prove dedup.
Notable findings / supporting changes
class_wiki_pages/class_wiki_engagementwon't run unless both connectors' bronze tables exist. This PR adds placeholders for both Outline (4 streams) and Confluence (6 streams), even though each spec seeds only its own.bronze_jira.jira_userwas missingtenant_id—confluence__wiki_pages' identity join (accountId → email) reads it, so the model failed to compile on a fresh cluster. Added it (Nullable, so connectors seedingjira_userwithout it still load). Latent placeholder gap, not specific to these tests.parent_comment_id(the reply-integrity singular tests undertests/wiki/reference it).person_idfalls back toauthor_id; specs setauthor_idto the seeded person's email so identity resolves to a team member (for the team median).class_wiki_pages/class_wiki_engagement/class_wiki_activityat session start (incrementaldelete+insertunions both connectors → warm-rerun determinism; CI is fresh).Verification
./e2e.sh test -k "wiki_outline or wiki_confluence"→ 8 passed.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
tenant_idfor wiki-related Jira placeholders.Tests