Repository navigation
Conversation
Sync OpenRouter's public /datasets/session-cost dataset: weekly median cost per session by harness, model and turn-count range, with the snapshot meta copied onto each row. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: b63918f5-00b6-4cb4-975f-faddb3d77e98
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
Gilbert09
added
the
stamphog
Talyn App
|
|
🤖 CI report
|
| File | Comment lines | Added lines |
|---|---|---|
products/warehouse_sources/backend/temporal/data_imports/sources/openrouter/settings.py |
3 | 15 |
products/warehouse_sources/backend/temporal/data_imports/sources/openrouter/openrouter.py |
1 | 8 |
products/warehouse_sources/backend/temporal/data_imports/sources/openrouter/tests/test_openrouter.py |
1 | 18 |
This check does not block merging. It updates on every push and clears when the share drops.
There was a problem hiding this comment.
Reviewed the PR description, diff, existing discussion, endpoint configuration, metadata flattening, pagination/resume behavior, source integration, and added test against OpenRouter's API reference. Checked for correctness, security, data safety, performance, missing coverage, overengineering, and scope creep. Tests were not run as requested.
P1
- Pagination-cap exhaustion is treated as a successful full refresh. If the dataset contains more than 5,500 cells, the sync stops after the page at offset 5,000 and publishes an incomplete table. The documented maximum offset limits requests, not the total dataset size. Fail safely at the cap (as the batches paginator does), or partition requests using the supported filters to retrieve the remaining cells. The new test currently requires successful termination at this boundary without establishing that the dataset is exhausted.
No P0 or P2 issues found.
Inline comments
products/warehouse_sources/backend/temporal/data_imports/sources/openrouter/openrouter.py:215— P1: This break reports successful exhaustion when the page at offset 5000 is full. If the snapshot has more than 5500 cells, every subsequent cell is omitted and the full refresh publishes an incomplete table; the API's offset ceiling does not bound the dataset size. Please raise OpenRouterPageCapError at this boundary, like the batches paginator, unless completeness can be established, or use the supported filters to paginate disjoint subsets. Add coverage that prevents an exhausted pagination cap from silently succeeding.
— Robo Bill v2 (gpt-6.1-sol, high reasoning)
![posthog[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 58e6c2d: 1 Should fix.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
![stamphog[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
Owning-team authorship, strong familiarity, and a current-head approval provide assurance, but @PostHog's substantive pagination concern remains unfixed. Address that comment in openrouter.py and request follow-up review from @PostHog/team-warehouse-sources before re-requesting.
- Author wrote 100% of the modified lines and has 23 merged PRs in these paths (familiarity STRONG).
- PostHog Review (Standard) reviewed the current head: 1 finding, posted on the PR.
- bill-ph, posthog[bot] reviewed the current head.
- @PostHog, products/warehouse_sources/backend/temporal/data_imports/sources/openrouter/openrouter.py: The pagination-cap concern remains unresolved. Lines 212–215 terminate successfully after a full page at offset 5000 without establishing completeness; source.py exposes this dataset as full-refresh only, so additional rows can be silently omitted. Fail the import when completeness cannot be established and update the test that currently expects successful truncation.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 42L, 4F substantive, 62L/6F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (62L, 6F, single-area, feat) |
| stamphog 2.6.1 | .stamphog/policy.yml @ 58e6c2d · reviewed head 58e6c2d |
HostHog preview —
|
… cap A full page at the API's maximum offset can't prove the dataset is exhausted, so raise instead of publishing a truncated full refresh, matching the batches paginator. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: b63918f5-00b6-4cb4-975f-faddb3d77e98
|
Robo Bill's P1 on |
There was a problem hiding this comment.
Approved.
The risky ingestion change has assurance through owning-team authorship and STRONG familiarity. The older pagination concern is fixed in the current code and covered by a boundary regression test; no outstanding substantive concerns remain.
- Author wrote 100% of the modified lines and has 23 merged PRs in these paths (familiarity STRONG).
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 44L, 4F substantive, 65L/6F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (65L, 6F, single-area, feat) |
| stamphog 2.6.1 | .stamphog/policy.yml @ db2f793 · reviewed head db2f793 |
Problem
Changes
session_coststable holds the weekly median USD cost per session, per harness, model and turn-count range.metafields.How did you test this code?
Test rationale: a new transport test catches dropped
metacolumns, and a sync that succeeds without reading every row past the offset cap.test_offset_limit_stops_on_short_pagecovers neither.Release status
Docs update
🤖 Agent context
Autonomy: Fully autonomous
Agent: Claude Code, Claude Opus 5.5 (
claude-opus-5-5)Skills:
/implementing-warehouse-sources,/writing-pr-descriptionsMore context for AI agents
app_slug,model_permaslug,turn_range. No server time filter, so no incremental sync.Created with PostHog Desktop from a PostHog Desktop task
🤖 Generated with Claude Code