feat(actions): HPA/VPA right-sizing audit with Datadog percentiles and Slack review card - #593
Merged
Conversation
Contributor
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughAdds Datadog ChangesDatadog metrics and query handling
Recommendation lifecycle
Right-sizing action and Slack tools
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Agent
participant send_hpa_vpa_recommendation
participant hpa_vpa_recommendations
participant Slack
Agent->>send_hpa_vpa_recommendation: submit right-sizing recommendation
send_hpa_vpa_recommendation->>hpa_vpa_recommendations: claim or refresh recommendation
send_hpa_vpa_recommendation->>Slack: post or update recommendation card
Slack-->>send_hpa_vpa_recommendation: return message timestamp
send_hpa_vpa_recommendation->>hpa_vpa_recommendations: attach Slack message
sequenceDiagram
participant Slack
participant _handle_hpa_vpa_dismiss
participant hpa_vpa_recommendations
participant close_pull_request
Slack->>_handle_hpa_vpa_dismiss: submit dismiss action
_handle_hpa_vpa_dismiss->>hpa_vpa_recommendations: transition recommendation to dismissed
_handle_hpa_vpa_dismiss->>close_pull_request: close linked pull request
close_pull_request-->>_handle_hpa_vpa_dismiss: return closure result
_handle_hpa_vpa_dismiss->>Slack: rewrite card and send status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![arvo-ai-staging[bot]](https://avatars.githubusercontent.com/in/4088260?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@server/chat/backend/agent/access/mode_access_controller.py`:
- Line 88: Define one class-level constant for the read-only deny-list and
replace the duplicated literals in both filter_tools and is_tool_allowed with
that shared constant, ensuring both Ask-mode checks remain synchronized.
In `@server/chat/backend/agent/skills/integrations/datadog/SKILL.md`:
- Line 30: Add a blank line immediately after each new Resource Types heading
and the other three affected ### headings in SKILL.md, before their body
content, to satisfy markdownlint MD022. Preserve the existing heading text and
surrounding content.
In `@server/chat/backend/agent/tools/hpa_vpa_card_tool.py`:
- Around line 363-416: The cooldown handling inside the main database workflow
should be extracted to reduce cognitive complexity. Add a helper named
_check_cooldown(cur, org_id, workload_key, severity, workload) returning the
suppression JSON string or None, move the existing cooldown lookup, superseding,
logging, and response construction into it, then call it before
get_live_recommendation and return immediately when it yields a response while
preserving claim/post ordering.
- Around line 483-492: The _update_existing flow must keep the recommendation’s
VCS provider consistent when refreshing an existing record. Update the
refresh_recommendation call and its implementation to propagate the current
vcs_provider, or validate it against live["vcs_provider"] before refreshing;
ensure repo_full_name and pr_number are never updated while the stored provider
remains stale.
In `@server/routes/slack/slack_events.py`:
- Around line 680-717: Add a programmatic Casbin permission check for
clicker_user_id in the Slack dismissal flow, using the same permission predicate
as the actions/PR surface, before calling dismiss_recommendation. When the check
fails, send the ephemeral “not permitted” response and return without dismissing
or closing the PR; preserve the existing organization-ownership validation and
use the established authorization helper rather than adding a new manual role
check.
- Around line 719-733: The already_merged branch must track whether mark_merged
successfully clears the existing cooldown. Update the mark_merged handling and
status_line construction so success reports that no cooldown was applied, while
an exception reports that the accepted change remains subject to the existing
cooldown; preserve the current logging and transaction behavior.
In `@server/services/actions/hpa_vpa_recommendations.py`:
- Around line 326-351: Update the dismissal query and result mapping around the
returned-field tuple and `keys` so the SQL `RETURNING` column list is generated
from the single `keys` definition, preserving the existing field order. Zip the
returned row with `keys` using strict length validation to raise on mismatches
instead of silently truncating.
In `@server/tests/chat/test_datadog_metric_stats.py`:
- Line 276: Split the composite assertions checking serialized output into
separate assertions for “Infinity” and “NaN” at each referenced location,
preserving the existing validation while making failures identify the specific
forbidden value.
In `@server/utils/db/db_utils.py`:
- Around line 1445-1457: Update the table definition in the database schema
initialization to use TIMESTAMPTZ for dismissed_at and cooldown_until,
preserving the existing defaults and indexes. Ensure the recommendation
write/read paths, including dismiss_recommendation, get_active_cooldown, and
list_recommendations, continue using timezone-aware UTC values consistently.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 52fccd80-e14d-46bc-8c1e-af2405285f49
📒 Files selected for processing (14)
server/chat/backend/agent/access/mode_access_controller.pyserver/chat/backend/agent/skills/integrations/datadog/SKILL.mdserver/chat/backend/agent/tools/cloud_tools.pyserver/chat/backend/agent/tools/datadog_tool.pyserver/chat/backend/agent/tools/github_rca_tool.pyserver/chat/backend/agent/tools/hpa_vpa_card_tool.pyserver/routes/slack/slack_events.pyserver/services/actions/hpa_vpa_action.pyserver/services/actions/hpa_vpa_recommendations.pyserver/services/actions/system_actions.pyserver/tests/chat/test_datadog_metric_stats.pyserver/tests/conftest.pyserver/tests/services/test_hpa_vpa_recommendations.pyserver/utils/db/db_utils.py
damianloch
reviewed
Jul 30, 2026
damianloch
requested changes
Jul 30, 2026
…tion, server-derived severity, Slack feedback gaps
…dialog, cap recommendation list payload
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@server/chat/backend/agent/skills/integrations/datadog/SKILL.md`:
- Around line 95-98: Update the fenced code block containing the Datadog metric
query examples to specify the text language tag, leaving the query expressions
unchanged.
In `@server/chat/backend/agent/tools/hpa_vpa_card_tool.py`:
- Around line 477-491: Update the recommendation-posting flow around the outer
try/except and _check_cooldown result so superseded_id is captured before
entering the try and remains available to error handling. In the outer exception
handler, call _restore_cooldown with the active connection/cursor and
superseded_id before returning the error response, while preserving the existing
_post_new and _post_new_for_existing compensation behavior.
In `@server/services/actions/hpa_vpa_recommendations.py`:
- Around line 244-250: Sanitize the untrusted dimension value in the
mismatched-units warning within the HpaVpa recommendations logic. Update the
logger.warning call to pass dimension through the module’s existing sanitize
helper while preserving the current message and value arguments.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: b0002f10-fd9e-494c-8737-cc250f1dccbb
📒 Files selected for processing (15)
server/chat/backend/agent/access/mode_access_controller.pyserver/chat/backend/agent/skills/integrations/datadog/SKILL.mdserver/chat/backend/agent/tools/cloud_tools.pyserver/chat/backend/agent/tools/datadog_tool.pyserver/chat/backend/agent/tools/hpa_vpa_card_tool.pyserver/chat/backend/agent/utils/state.pyserver/chat/background/task.pyserver/routes/slack/slack_events.pyserver/services/actions/hpa_vpa_action.pyserver/services/actions/hpa_vpa_recommendations.pyserver/tests/chat/test_datadog_metric_stats.pyserver/tests/chat/test_hpa_vpa_card_blocks.pyserver/tests/conftest.pyserver/tests/services/test_hpa_vpa_recommendations.pyserver/utils/db/db_utils.py
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
damianloch
approved these changes
Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a scheduled action that compares real Datadog usage against the CPU/memory requests, limits, and HPA
maxReplicasdeclared in a team's IaC, opens one PR per materially mis-sized workload, and posts a Slack card with View PR and Dismiss. Aurora never applies anything — the PR is the change and a human merges it.Seeded
enabled=False, so nothing runs on a schedule until someone turns it on.What's here
metric_statsDatadog resource type — Datadog has no time-percentile capability (every documented route is rejected or silently returns zero series, and both target metrics are gauges), so p95 is computed in Python from rolled-up points, behind a_P95_BY_SOURCEdispatch seam with an import-time guard. Returns one compact row per series rather than a single fat dict:_truncate_resultskeeps a prefix, so today a 973 KB metrics payload becomescount: 0, which is indistinguishable from "no data" and would lead the agent to cut an idle-looking workload. Budgeted at 30 KB againstPASS_THROUGH_CHARS(40 K), notMAX_OUTPUT_SIZE(120 K) — above that an LLM summarizer paraphrases percentiles into plausible fiction.hpa_vpa_recommendationstable + RLS, with a partial unique index enforcing at most one live proposal per workload.close_pull_requestas a provider dispatch (GitHub implemented; GitLab/Bitbucket are a table entry each, not a migration — hence thevcs_providercolumn).send_hpa_vpa_recommendation(first agent-callable Slack write tool in the repo) andlist_hpa_vpa_recommendations, so the prompt checks cooldown state before opening a PR.Design decisions worth review
merged, no cooldown, or the next genuine drift goes unreported.requests.patchinserver/. MCPupdate_pull_requestis allowlisted inmcp_tools.pybut absent fromtool_registry.py, sogate_actiondenies it in background context. Direct REST is the only reliable path.tool_registry.pyentry and nogate_actioncall on the card tool. It writes only to Aurora's own RLS table and the channel Aurora created; a registry key would be a dead switch (_is_org_tool_permittedis only consulted insidegate_action, which this native tool never calls), andgate_actiondenies unconditionally in scheduled runs. The working kill switch isenabled=Falseon the action.resource_type='metrics'behaviour is unchanged. Its output contract predates this work and the RCA prompts depend on it; onlymetric_statsauto-picks an interval. A test pins that boundary.max(not p95) because the failure mode is an OOM-kill; CPU is symmetric because throttling is recoverable.Verification
tests/architectural/unchanged at 67).successruns, and the agent independently builtsum:kubernetes.{cpu.usage.total,memory.usage}{*} by {kube_deployment}— the intendedsum-across-pods shape — then wrote the living document and correctly issued no recommendations rather than inventing any.dismissed_atunchanged; merge clears the cooldown; a duplicate live claim is blocked by the partial unique index./slack/interactions: valid signature → 200, bad signature → 403, stale timestamp → 403, malformed UUID → 200 with a clean message.Two bugs found in review and fixed with regression tests: a NaN metric point silently broke
list.sort()and understatedmax(which guards the OOM rule), and Dismiss closed the PR as the clicker rather than the account that opened it, which would have failed for any org-mate without their own GitHub connection.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Replaces #592, which GitHub auto-closed when the branch was renamed. Same commits, same head SHA (
693be8c6); no code changed.