Skip to content

feat(github-app-playground): bump chart to v0.9.0 / appVersion 1.5.0 - #26

Merged
chrisleekr merged 2 commits into
mainfrom
feat/github-app-playground-v0.9.0-bump
Apr 26, 2026
Merged

feat(github-app-playground): bump chart to v0.9.0 / appVersion 1.5.0#26
chrisleekr merged 2 commits into
mainfrom
feat/github-app-playground-v0.9.0-bump

Conversation

@chrisleekr

@chrisleekr chrisleekr commented Apr 26, 2026

Copy link
Copy Markdown
Owner

Summary

Bumps github-app-playground Helm chart to v0.9.0 with appVersion 1.5.0 (chrisleekr/github-app#63 — cascade PR retargeting + bounded review/resolve loop).

Upstream v1.5.0 fixes the ship pipeline cascade: review and resolve were inheriting the parent ship's issue target and bailing immediately because they require a PR target. v1.5.0 retargets cascade children to the PR opened by implement, and runs a bounded review→resolve loop that exits cleanly when findings reach zero (or hits the cap and emits a manual-re-review hint).

This chart bump exposes the one new tunable upstream introduced — REVIEW_RESOLVE_MAX_ITERATIONS (int, range 1–5, upstream default 2) — as config.reviewResolveMaxIterations in values.yaml. It is gated by the same ne … "" non-empty check used by the sibling cap tunables (AGENT_MAX_TURNS, DEFAULT_MAXTURNS), so leaving it empty omits the env var entirely and the upstream Zod default takes effect.

Chart minor bump (0.8.1 → 0.9.0) is appropriate because we are adding a new, optional, backward-compatible values knob — not just an appVersion bump.

Before

flowchart LR
  values["values.yaml<br/>config.*"]:::cur
  cm["templates/configmap.yaml<br/>Groups 1–10"]:::cur
  pod["pod env"]:::cur
  zod["upstream Zod schema<br/>reviewResolveMaxIterations<br/>default 2 — unreachable"]:::miss

  values --> cm --> pod --> zod

  classDef cur fill:#1f3a5f,color:#ffffff,stroke:#ffffff
  classDef miss fill:#922b21,color:#ffffff,stroke:#ffffff
Loading

After

flowchart LR
  values["values.yaml<br/>config.reviewResolveMaxIterations<br/>empty by default"]:::add
  gate{"value != empty?"}:::add
  emit["REVIEW_RESOLVE_MAX_ITERATIONS<br/>quoted"]:::add
  skip["env var not emitted"]:::cur
  pod["pod env"]:::cur
  zod["upstream Zod<br/>z.coerce.number.int<br/>1–5, default 2"]:::cur

  values --> gate
  gate -- "yes" --> emit --> pod --> zod
  gate -- "no" --> skip --> pod --> zod

  classDef cur fill:#1f3a5f,color:#ffffff,stroke:#ffffff
  classDef add fill:#196f3d,color:#ffffff,stroke:#ffffff
Loading

Changes

  • Chart.yaml — chart 0.8.1 → 0.9.0, appVersion "1.4.0" → "1.5.0". Replaced previous artifacthub.io/changes entry with two new entries: a changed entry describing the upstream behaviour change, and an added entry describing the new tunable.
  • values.yaml — added Group 12 section with reviewResolveMaxIterations: "" (empty default = use upstream default of 2). Range and behaviour documented inline.
  • templates/configmap.yaml — added Group 12 section emitting REVIEW_RESOLVE_MAX_ITERATIONS only when the value is non-empty, matching the gate-and-quote pattern of sibling cap tunables AGENT_MAX_TURNS (line 41) and DEFAULT_MAXTURNS (line 93). No int cast — upstream Zod uses z.coerce.number().int() which already handles the string→int coercion.

Verification

  • helm lint charts/github-app-playground — clean (1 INFO about icon, pre-existing).
  • helm template charts/github-app-playground (default values) — REVIEW_RESOLVE_MAX_ITERATIONS not present in rendered ConfigMap (upstream Zod default of 2 takes effect).
  • helm template charts/github-app-playground --set config.reviewResolveMaxIterations=4 — renders REVIEW_RESOLVE_MAX_ITERATIONS: "4" (quoted string, as required by ConfigMap).

@coderabbitai

coderabbitai Bot commented Apr 26, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The pull request updates the GitHub app playground Helm chart from version 0.8.1 to 0.9.0, bumping the application version to 1.5.0. A new optional configuration parameter reviewResolveMaxIterations is introduced across the chart's values, ConfigMap template, and changelog to allow operators to cap review→resolve loop iterations.

Changes

Cohort / File(s) Summary
Helm Chart Versioning & Changelog
charts/github-app-playground/Chart.yaml
Updates chart version to 0.9.0 and appVersion to 1.5.0. Updates Artifact Hub changelog to document the version bump, upstream behavior changes (cascade PR retargeting, bounded review→resolve loop), and new reviewResolveMaxIterations config option.
Configuration Schema & Values
charts/github-app-playground/values.yaml, charts/github-app-playground/templates/configmap.yaml
Adds new optional config.reviewResolveMaxIterations parameter to values schema with documentation (defaults to upstream value 2 when unset; valid range 1–5). Extends ConfigMap template to render REVIEW_RESOLVE_MAX_ITERATIONS env var when the value is non-empty, coercing it to an integer.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Poem

🐰 A rabbit hops through config land,
Where values flow with gentle hand.
New reviewResolveMaxIterations blooms—
A bounded loop that tidies rooms!
From 0.8 to 0.9 we leap,
Changes small, yet oh so deep. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main changes: chart version bump from 0.8.1 to 0.9.0 and appVersion bump from 1.4.0 to 1.5.0, which are the primary version updates reflected throughout the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/github-app-playground-v0.9.0-bump

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
charts/github-app-playground/templates/configmap.yaml (1)

95-99: Minor consistency nit: int cast diverges from sibling tunables.

The PR notes mention reusing "the same non-empty check and int-cast pattern used for existing DEFAULT_MAXTURNS/AGENT_MAX_TURNS", but those two lines actually emit {{ $c.X | quote }} without an int cast (lines 41 and 93). Only the *_MS/threshold values use int … | quote.

The current behaviour is fine — int … | quote is arguably the safer form — but if you want the three "cap"-style tunables to be uniform, either drop the int here or add it to the sibling lines in a follow-up. Silent truncation also differs: a stray "2.5" here would become "2", whereas the siblings would pass it through and let Zod reject. Either choice is defensible; just flagging the divergence.

♻️ Option A — drop the int cast to match siblings
   {{- if ne ($c.reviewResolveMaxIterations | toString) "" }}
-  REVIEW_RESOLVE_MAX_ITERATIONS: {{ int $c.reviewResolveMaxIterations | quote }}
+  REVIEW_RESOLVE_MAX_ITERATIONS: {{ $c.reviewResolveMaxIterations | quote }}
   {{- end }}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@charts/github-app-playground/templates/configmap.yaml` around lines 95 - 99,
The REVIEW_RESOLVE_MAX_ITERATIONS entry uses an int cast that diverges from the
sibling cap tunables (DEFAULT_MAXTURNS and AGENT_MAX_TURNS); make them
consistent by removing the int cast so REVIEW_RESOLVE_MAX_ITERATIONS is emitted
as {{ $c.reviewResolveMaxIterations | quote }} under the same non-empty check,
matching the other cap-style tunables and avoiding silent truncation differences
between these keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@charts/github-app-playground/templates/configmap.yaml`:
- Around line 95-99: The REVIEW_RESOLVE_MAX_ITERATIONS entry uses an int cast
that diverges from the sibling cap tunables (DEFAULT_MAXTURNS and
AGENT_MAX_TURNS); make them consistent by removing the int cast so
REVIEW_RESOLVE_MAX_ITERATIONS is emitted as {{ $c.reviewResolveMaxIterations |
quote }} under the same non-empty check, matching the other cap-style tunables
and avoiding silent truncation differences between these keys.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: df31176d-51d9-402c-a390-5a79ee14924e

📥 Commits

Reviewing files that changed from the base of the PR and between f2718c3 and 3cb528b.

📒 Files selected for processing (3)
  • charts/github-app-playground/Chart.yaml
  • charts/github-app-playground/templates/configmap.yaml
  • charts/github-app-playground/values.yaml

@chrisleekr

Copy link
Copy Markdown
Owner Author

Thanks @coderabbitai — verified and applied. AGENT_MAX_TURNS (line 41) and DEFAULT_MAXTURNS (line 93) both emit without an int cast, so REVIEW_RESOLVE_MAX_ITERATIONS should match. Upstream Zod uses z.coerce.number().int().min(1).max(5) — the int cast in the chart added no real safety. Fixed in d5585e5 (Option A). PR description updated.

@coderabbitai

coderabbitai Bot commented Apr 26, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@chrisleekr: Thanks for the thorough verification and the clear explanation! Removing the int cast is the right call — since upstream Zod already coerces with z.coerce.number().int().min(1).max(5), the cast in the chart template was redundant and inconsistent with how AGENT_MAX_TURNS and DEFAULT_MAXTURNS are emitted. The fix in d5585e5 correctly aligns REVIEW_RESOLVE_MAX_ITERATIONS with the established pattern. 🎉


✏️ Learnings added
Learnt from: chrisleekr
URL: https://github.com/chrisleekr/helm-charts/pull/26

Timestamp: 2026-04-26T11:55:30.904Z
Learning: In the `charts/github-app-playground` Helm chart, env vars derived from `config.*` values are emitted as plain quoted strings without an `int` cast in the configmap template (e.g. `AGENT_MAX_TURNS` at line 41 and `DEFAULT_MAXTURNS` at line 93). Upstream validation/coercion (Zod) handles type enforcement, so the chart should not add redundant `int` casts for numeric config values.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@chrisleekr
chrisleekr merged commit 5ed2725 into main Apr 26, 2026
2 checks passed
@chrisleekr
chrisleekr deleted the feat/github-app-playground-v0.9.0-bump branch April 26, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant