Conversation
|
Risk: No findings The delta since the last review is purely mechanical: Sentinel reviewed |
|
Hey @Gilbert09! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
🤖 CI report
|
| First copy | Second copy | Lines | Tokens |
|---|---|---|---|
products/warehouse_sources/backend/temporal/data_imports/sources/acculynx/source.py:18 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_config/source.py:22 |
14 | 137 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_budgets/source.py:140 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_config/source.py:110 |
27 | 100 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_config/aws_config.py:103 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_glue_data_catalog/aws_glue_data_catalog.py:111 |
13 | 90 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_config/aws_config.py:105 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_waf/aws_waf.py:112 |
12 | 74 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_cloudtrail/source.py:142 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_config/source.py:117 |
21 | 73 |
✅ Duplication (TypeScript) — clean
New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
⚠️ Backend coverage — 98.0% of changed backend lines covered — 5 uncovered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): ████████████████████ 98.0% (401 / 406)
| File | Patch | Uncovered changed lines |
|---|---|---|
products/warehouse_sources/backend/temporal/data_imports/sources/aws_config/source.py |
88.5% | 76, 79, 87 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_config/aws_config.py |
98.7% | 65–66 |
🤖 Agents: add a test only if an uncovered line exposes a realistic regression that existing tests miss. Otherwise explain why no new test is needed under "How did you test this code?". Gap list: the patch-coverage artifact on this run (gh run download 39491575517581 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
demo |
███████████░░░░░░░░░ 53.4% |
1,445 / 2,707 |
batch_exports |
████████████████░░░░ 81.3% |
21,584 / 26,561 |
cdp |
██████████████████░░ 88.3% |
4,559 / 5,164 |
mcp_analytics |
██████████████████░░ 89.2% |
5,038 / 5,651 |
product_tours |
██████████████████░░ 89.3% |
1,340 / 1,500 |
dashboards |
██████████████████░░ 89.6% |
6,924 / 7,727 |
notebooks |
██████████████████░░ 90.2% |
15,514 / 17,207 |
signals |
██████████████████░░ 90.4% |
60,161 / 66,532 |
cohorts |
██████████████████░░ 90.5% |
8,534 / 9,434 |
data_warehouse |
██████████████████░░ 90.6% |
14,375 / 15,860 |
streamlit_apps |
██████████████████░░ 90.8% |
2,684 / 2,956 |
managed_warehouse |
██████████████████░░ 91.0% |
10,252 / 11,263 |
data_modeling |
██████████████████░░ 91.2% |
10,562 / 11,584 |
tasks |
██████████████████░░ 91.3% |
79,682 / 87,258 |
exports |
██████████████████░░ 91.7% |
9,684 / 10,566 |
business_knowledge |
██████████████████░░ 92.0% |
8,472 / 9,208 |
engineering_analytics |
██████████████████░░ 92.2% |
11,497 / 12,475 |
today |
██████████████████░░ 92.3% |
999 / 1,082 |
early_access_features |
███████████████████░ 92.6% |
1,339 / 1,446 |
conversations |
███████████████████░ 92.6% |
29,225 / 31,559 |
stamphog |
███████████████████░ 92.8% |
8,109 / 8,742 |
canvas |
███████████████████░ 92.9% |
7,155 / 7,703 |
approvals |
███████████████████░ 93.0% |
3,974 / 4,271 |
mcp_registry |
███████████████████░ 93.1% |
1,670 / 1,794 |
notifications |
███████████████████░ 93.2% |
1,144 / 1,228 |
error_tracking |
███████████████████░ 93.3% |
16,389 / 17,573 |
surveys |
███████████████████░ 93.4% |
6,644 / 7,113 |
autoresearch |
███████████████████░ 93.4% |
8,837 / 9,457 |
slack_app |
███████████████████░ 93.7% |
14,611 / 15,600 |
context_layer |
███████████████████░ 93.8% |
3,415 / 3,639 |
web_analytics |
███████████████████░ 93.9% |
23,680 / 25,229 |
billing_alerts |
███████████████████░ 94.1% |
2,094 / 2,226 |
mcp_store |
███████████████████░ 94.3% |
8,959 / 9,501 |
alerts |
███████████████████░ 94.7% |
9,319 / 9,844 |
wizard |
███████████████████░ 94.7% |
6,150 / 6,496 |
workflows |
███████████████████░ 94.7% |
15,187 / 16,034 |
ai_observability |
███████████████████░ 94.7% |
26,037 / 27,485 |
reminders |
███████████████████░ 94.8% |
760 / 802 |
review_hog |
███████████████████░ 95.0% |
11,750 / 12,362 |
annotations |
███████████████████░ 95.1% |
817 / 859 |
endpoints |
███████████████████░ 95.1% |
9,234 / 9,706 |
customer_analytics |
███████████████████░ 95.2% |
26,116 / 27,428 |
marketing_analytics |
███████████████████░ 95.3% |
19,450 / 20,413 |
posthog_ai |
███████████████████░ 95.4% |
2,530 / 2,653 |
actions |
███████████████████░ 95.5% |
756 / 792 |
logs |
███████████████████░ 95.5% |
15,468 / 16,200 |
experiments |
███████████████████░ 95.5% |
33,000 / 34,548 |
data_catalog |
███████████████████░ 95.5% |
4,401 / 4,606 |
tracing |
███████████████████░ 95.6% |
3,536 / 3,699 |
replay_vision |
███████████████████░ 95.6% |
29,389 / 30,727 |
growth |
███████████████████░ 95.7% |
11,381 / 11,888 |
skills |
███████████████████░ 95.8% |
6,972 / 7,274 |
messaging |
███████████████████░ 95.9% |
3,834 / 3,999 |
product_analytics |
███████████████████░ 96.0% |
28,521 / 29,696 |
revenue_analytics |
███████████████████░ 96.4% |
1,889 / 1,959 |
user_interviews |
███████████████████░ 96.5% |
2,870 / 2,974 |
feature_flags |
███████████████████░ 96.6% |
27,119 / 28,060 |
access_control |
███████████████████░ 96.7% |
7,739 / 8,007 |
warehouse_sources |
███████████████████░ 97.3% |
473,150 / 486,234 |
data_quality |
████████████████████ 97.5% |
7,701 / 7,895 |
metrics |
████████████████████ 98.0% |
4,252 / 4,338 |
analytics_platform |
████████████████████ 98.3% |
2,784 / 2,833 |
pulse |
████████████████████ 98.5% |
2,046 / 2,078 |
live_debugger |
████████████████████ 99.2% |
626 / 631 |
Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (5)📝 WalkthroughWalkthroughThe change implements AWS Config ingestion for four datasets. It adds endpoint and credential configuration, SigV4-signed requests, error handling, row normalization, and resumable pagination. The source now provides schema discovery, credential validation, and setup metadata. Tests cover requests, pagination, resume behavior, and error handling. The source inventory lists AWS Config as implemented. Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to If an AWS Config resume token expires, earlier pages can be added to the warehouse again, duplicating records. Fix the recovery path before merging to protect the accuracy of synced data. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
A new stamphog review started for this PR — the fresh verdict replaces this approval.
|
CI is green on 🦉 via talyn.dev |
|
/trunk merge |
Rebase the AWS Config source implementation onto current master and preserve newly added AWS source inventory entries while resolving SOURCES.md.
65c81ae to
05f877e
Compare
A new stamphog review started for this PR — the fresh verdict replaces this approval.
There was a problem hiding this comment.
Approved.
This is a self-contained new warehouse source written by an owning-team author with strong familiarity, and it has thorough tests. Credential handling looks sound: the region is regex-validated before it goes into the host, redirects are disabled, secrets are redacted, and the secret fields are marked secret.
- Author wrote 88% of the modified lines and has 95 merged PRs in these paths (familiarity STRONG).
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 510L, 5F substantive, 903L/8F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (903L, 8F, single-area, feat) |
| stamphog 2.3.1 | .stamphog/policy.yml @ 05f877e · reviewed head 05f877e |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟠 Major · Abort the resumed attempt after clearing an expired token. · aws_config.py:172-193
products/warehouse_sources/backend/temporal/data_imports/sources/aws_config/aws_config.py:172-193
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAbort the resumed attempt after clearing an expired token.
This branch currently restarts inside the same pipeline. V2 and V3 have already captured their resume flags, so the replayed full-refresh rows append to the existing table.
Clear the checkpoint and re-raise the error. The import workflow retries this activity up to three times. The next attempt sees no resume state, overwrites the first page, and then appends later pages without skipping source data.
Suggested fix
if restarting and error.code == "InvalidNextTokenException": + manager.clear_state() - next_token = None - restarting = False - continue + raise
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 61deaae0-f152-4204-a603-8c30e0edfc9b
📒 Files selected for processing (1)
products/warehouse_sources/backend/temporal/data_imports/sources/SOURCES.md
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 2 remain after this review.
|
/trunk merge |
Problem
Changes
resourcesconfig_rulesrule_complianceconformance_packsaws_organizationssource.unreleasedSourceremoved.SOURCES.mdrow.How did you test this code?
Test rationale: the source is new, so no existing test covers its transport.
Release status
Automatic notifications
Docs update