Conversation
|
✨ Submitted to Merge by Tom Owers (@Gilbert09). It will be added to the merge queue once all branch protection rules pass and impacted targets have been uploaded. See more details here. |
Gilbert09
added
the
stamphog
Talyn App
|
Risk: No findings This delta is typing-only: a list annotation on the findings hydration buffer in aws_macie.py and Sentinel reviewed |
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
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_macie/source.py:24 |
14 | 137 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_batch/source.py:109 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_macie/source.py:113 |
26 | 94 |
✅ 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% (380 / 385)
| File | Patch | Uncovered changed lines |
|---|---|---|
products/warehouse_sources/backend/temporal/data_imports/sources/aws_macie/source.py |
92.3% | 78, 81 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_macie/aws_macie.py |
97.8% | 104–105, 107 |
🤖 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 436337654704227 -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,582 / 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,186 / 66,556 |
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,745 / 87,306 |
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 |
wizard |
███████████████████░ 94.7% |
6,150 / 6,496 |
alerts |
███████████████████░ 94.7% |
9,320 / 9,844 |
workflows |
███████████████████░ 94.7% |
15,187 / 16,034 |
ai_observability |
███████████████████░ 94.7% |
26,034 / 27,482 |
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.6% |
4,402 / 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,761 / 486,846 |
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. Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: PostHog/posthog/.coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughAdds an AWS Macie source for findings, buckets, classification jobs, and member accounts. The source uses signed API requests, resumable pagination, findings timestamp filtering, and batched findings hydration. It also adds credential validation, source configuration, endpoint schemas, canonical descriptions, and tests. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The change adds resumable AWS Macie ingestion. The inspected framework contract prevents advancing the saved cursor before its rows are written, leaving no concrete merge-blocking risk established here. 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.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Check every Macie endpoint during unscoped validation. · aws_macie.py:182-205
products/warehouse_sources/backend/temporal/data_imports/sources/aws_macie/aws_macie.py:182-205
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCheck every Macie endpoint during unscoped validation.
When
schema_nameis unset,validate_credentialsprobes onlyfindingsand treats access denial as success. One-shot setup then enables the Macie schemas, and credentials denied on every endpoint fail on the first sync. Probe each endpoint, continue after access-denied responses, and fail only when all endpoints deny access. This preserves configurations with access to at least one endpoint.Suggested fix
if endpoint not in MACIE_ENDPOINTS: return False, f"Unknown AWS Macie table: {endpoint}" + endpoints = [endpoint] if schema_name is not None else list(MACIE_ENDPOINTS) try: client = AwsMacieClient(config, api_version) except ValueError as error: return False, str(error) try: - page = client.request(MACIE_ENDPOINTS[endpoint].operation, list_payload(endpoint, None, 1)) - if schema_name == "findings" and page.get("findingIds"): - client.request("GetFindings", {"findingIds": page["findingIds"][:1]}) - except AwsMacieError as error: - if schema_name is None and error.code in {"AccessDenied", "AccessDeniedException"}: - return True, None - for pattern, message in ERROR_MESSAGES.items(): - if pattern.lower() in str(error).lower(): - return False, message - raise + accessible = False + for endpoint_name in endpoints: + try: + page = client.request( + MACIE_ENDPOINTS[endpoint_name].operation, list_payload(endpoint_name, None, 1) + ) + accessible = True + if schema_name == "findings" and page.get("findingIds"): + client.request("GetFindings", {"findingIds": page["findingIds"][:1]}) + except AwsMacieError as error: + if schema_name is None and error.code in {"AccessDenied", "AccessDeniedException"}: + continue + for pattern, message in ERROR_MESSAGES.items(): + if pattern.lower() in str(error).lower(): + return False, message + raise + if schema_name is None and not accessible: + return False, ERROR_MESSAGES["AccessDenied"] finally: client.close()
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 8301bd90-7002-4a89-a413-e6d9c7e98b45
📒 Files selected for processing (2)
products/warehouse_sources/backend/temporal/data_imports/sources/aws_macie/aws_macie.pyproducts/warehouse_sources/backend/temporal/data_imports/sources/aws_macie/tests/test_aws_macie.py
Limit details: You’ve used all 12 included reviews currently available.
|
CI is fully green and there are no unresolved review threads. The PR is cleanly mergeable, but GitHub still reports it as blocked with the Warehouse Sources team requested; a human review/approval from that team is needed to clear the remaining merge gate. 🦉 via talyn.dev |
|
/trunk merge |
ed3a52b to
8673808
Compare
A new stamphog review started for this PR — the fresh verdict replaces this approval.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
/trunk merge |
1 similar comment
|
/trunk merge |
A new stamphog review started for this PR — the fresh verdict replaces this approval.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
The merge conflict is resolved and the branch was refreshed against 🦉 via talyn.dev |
Resolve the SOURCES.md conflict with current master while preserving the source implementation.
d40ecda to
7fbaa27
Compare
A new stamphog review started for this PR — the fresh verdict replaces this approval.
There was a problem hiding this comment.
Approved.
New warehouse source following the existing aws_organizations pattern, authored by a STRONG-familiarity owning-team member, with thorough tests. Credentials are handled safely: the region is validated before use, redirects are off, and secrets are redacted. No showstoppers found.
- Author wrote 88% of the modified lines and has 89 merged PRs in these paths (familiarity STRONG).
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 508L, 5F substantive, 854L/7F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (854L, 7F, single-area, feat) |
| stamphog 2.3.1 | .stamphog/policy.yml @ 7fbaa27 · reviewed head 7fbaa27 |
|
/trunk merge |
This branch has not been deployed
1 participant
Problem
Changes
findingsbucketsclassification_jobsmembersaws_organizationssource.macie2).sort_mode="desc", because AWS does not return rows in one global order. The watermark then saves once at job end.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