Conversation
mark-vass-konghq
added
the
devex-improvement
📝 WalkthroughWalkthroughIntroduces a new PR pre-check skill definition that specifies architectural, code quality, and safety review protocols for pull request validation against OpenMeter standards. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.agents/skills/pr-precheck/SKILL.md (1)
16-22: Consider referencing the existing/serviceskill for patterns.This architectural guidance looks solid! Just a thought—since there's already a
/serviceskill that documents service/adapter patterns, constructors, DI wiring, and other architectural conventions, you might want to reference it here to avoid duplicating or diverging from that guidance over time.💡 Optional improvement
You could add a note like:
## 1. Architectural Layers & Logic Placement OpenMeter relies on a highly optimized stack for real-time event ingestion and billing. Ensure changes respect the following boundaries: + +> **Note:** For detailed service/adapter patterns, constructors, and DI wiring conventions, see the `/service` skill documentation. + * **API Layer (TypeSpec/Handlers):** Must remain thin. Complex business logic should not reside in API handlers.Based on learnings that reference the
/serviceskill for Go architectural patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/pr-precheck/SKILL.md around lines 16 - 22, The guidance in SKILL.md should reference the existing /service skill to avoid duplication; update the Architectural Layers & Logic Placement section to add a brief pointer mentioning the /service skill (or its canonical name) for established service/adapter patterns, constructors, and DI wiring so readers follow a single source of truth; locate the section under "Architectural Layers & Logic Placement" and insert a short sentence like "See /service for service/adapter patterns, constructors, and DI conventions" to ensure consistency with existing Go architectural patterns.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/pr-precheck/SKILL.md:
- Line 27: Update the Meters aggregation list in the SKILL.md documentation so
it includes the two missing supported types: UNIQUE_COUNT and LATEST;
specifically change the line that currently lists "SUM, COUNT, AVG, MIN, MAX" to
list "SUM, COUNT, AVG, MIN, MAX, UNIQUE_COUNT, LATEST" so the pre-check reflects
all seven supported aggregation types from the meter module.
---
Nitpick comments:
In @.agents/skills/pr-precheck/SKILL.md:
- Around line 16-22: The guidance in SKILL.md should reference the existing
/service skill to avoid duplication; update the Architectural Layers & Logic
Placement section to add a brief pointer mentioning the /service skill (or its
canonical name) for established service/adapter patterns, constructors, and DI
wiring so readers follow a single source of truth; locate the section under
"Architectural Layers & Logic Placement" and insert a short sentence like "See
/service for service/adapter patterns, constructors, and DI conventions" to
ensure consistency with existing Go architectural patterns.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 65e316fa-0ea5-47d7-8bc4-dcffadfc8082
📒 Files selected for processing (1)
.agents/skills/pr-precheck/SKILL.md
| ## 2. Domain & Data Patterns | ||
| * **CloudEvents:** OpenMeter ingests events in the CloudEvents format. Any changes to event ingestion must strictly validate or adhere to the CloudEvents schema (`id`, `source`, `subject`, `type`, `time`, `data`). | ||
| * **Event Deduplication:** Ensure any ingestion modifications respect deduplication logic (handled via `id` and `source` fields). | ||
| * **Meters:** Check that aggregations use supported types (SUM, COUNT, AVG, MIN, MAX). |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify all supported meter aggregation types in the codebase
# Search for MeterAggregation constant definitions
ast-grep --pattern $'const (
$$$
)' --lang go | rg -A 20 'MeterAggregation'
# Also check the Values() method implementation
rg -A 15 'func \(MeterAggregation\) Values\(\)' --type goRepository: openmeterio/openmeter
Length of output: 8318
Meter aggregations list needs updating—missing UNIQUE_COUNT and LATEST.
The pre-check currently validates against only "SUM, COUNT, AVG, MIN, MAX", but the codebase actually supports 7 types. Missing from the list are UNIQUE_COUNT and LATEST, which are legitimate aggregation types defined in the meter module. This gaps means the skill won't catch code using these types, leading to false negatives.
Update the list to: SUM, COUNT, AVG, MIN, MAX, UNIQUE_COUNT, LATEST.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/pr-precheck/SKILL.md at line 27, Update the Meters
aggregation list in the SKILL.md documentation so it includes the two missing
supported types: UNIQUE_COUNT and LATEST; specifically change the line that
currently lists "SUM, COUNT, AVG, MIN, MAX" to list "SUM, COUNT, AVG, MIN, MAX,
UNIQUE_COUNT, LATEST" so the pre-check reflects all seven supported aggregation
types from the meter module.
1 participant
Add pr-precheck skill to agents. The purpose is improve your self-review skills and save time to not wait for coderabbit and CI.
Summary by CodeRabbit