Skip to content

Configure Pullfrog to run on Fireworks GLM 5.3 - #118

Closed
elviskahoro wants to merge 21 commits into
LineIndent:mainfrom
elviskahoro:conductor/fireworks-glm5.3-integration-fix
Closed

elviskahoro wants to merge 21 commits into
LineIndent:mainfrom
elviskahoro:conductor/fireworks-glm5.3-integration-fix

Conversation

@elviskahoro

Copy link
Copy Markdown
Collaborator

Problem

The Pullfrog workflow (run 37134011304) failed with "no API key found" — the repo had zero GitHub Actions secrets configured, and the workflow's env: block mapped none that existed.

Fix

Route Pullfrog runs to Fireworks GLM 5.3 via BYOK — the same model roborev uses for reviews in gtm-sdk (.roborev.toml: fireworks/accounts/fireworks/models/glm-5p3):

  • Pin the agent model on the pullfrog action's model input with a raw models.dev specifier: fireworks-ai/accounts/fireworks/models/glm-5p3 (per Pullfrog custom providers docs — any models.dev provider activates the moment its API key is in the run env).
  • Map FIREWORKS_API_KEY from GitHub Actions secrets into the run env: block.
  • FIREWORKS_API_KEY has been added as a repo secret (verified authenticating against api.fireworks.ai).

Required console step (manual)

In the Pullfrog console, set the repo model to Custom (set in workflow) — a model stored in the console overrides the workflow's model input, so this must be set for the workflow pin to take effect. Billing Model access should be BYOK.

detail-app Bot and others added 21 commits October 2, 2026 21:42
Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
…ed, toBase62) (#5)

Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
Fixes copy buttons in chart_util_wrapper and file_codeblock being
absolutely positioned without a positioned ancestor, which caused
them to anchor to the page root instead of the codeblock. Matches
the pattern already used in demo_wrapper. Closes AI-342.
Collapsible panels snap open and closed because the height state selectors have no transition. Match Accordion timing so both interactions animate consistently.
* chore: remove dead menu.py constants and type aliases

* Regenerate menu.md after dead-code removal

---------

Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
* chore(tools): add gtm-linear and issue-fetch CLI

gtm-linear is a Python SDK with no executable, so
scripts/linear_cli.py wraps LinearWorkflow as read-only
commands (viewer/teams/issues/issue/search) for fetching
Linear issues. Auth resolves LINEAR_API_KEY from env or
gitignored .env.local.

* refactor(tools): port Linear CLI to Typer

Type-hint-driven commands, enum state choices, rich --help,
typer.echo/secho output. Typer added as a dev dependency.

* fix(tools): harden Linear CLI after review

- Disable Typer pretty exceptions (locals in tracebacks could
  expose the API key)
- One _fail() error style for all failure paths
- Bound --limit to 1-100 via option validation instead of
  silent clamping
- Add CliRunner tests covering state enum, exit codes, auth
  failure, and LinearAPIError mapping

* test(tools): tighten Linear CLI tests after review

Assert main() prints the API error to stderr, cover
ValidationError handling with from_env patched, document
the verified settings sources on the missing-key test, use
result.output directly (click 8.2+ interleaves stderr), note
the --limit validation behavior in the CLI docstring.

* test(tools): finish Linear CLI test polish after review

Clear every LINEAR_* env var in the missing-key smoke test,
drop the version-pinned comment, add a case proving
unrelated from_env exceptions are not misreported.

* test(tools): match LINEAR_ env vars case-insensitively

pydantic-settings resolves env names case-insensitively;
also assert the original exception message survives and
document why ValueError guards a ValidationError regression.
…es (#23)

Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
The Pullfrog workflow failed with "no API key found" — no provider
credentials were configured in GitHub Actions secrets.

Route Pullfrog runs to Fireworks GLM 5.3 (the same model roborev uses
for reviews in gtm-sdk, accounts/fireworks/models/glm-5p3) via BYOK:

- Pin the agent model with a raw models.dev specifier
  (fireworks-ai/accounts/fireworks/models/glm-5p3) on the pullfrog
  action's `model` input.
- Map FIREWORKS_API_KEY from GitHub Actions secrets into the run env.

FIREWORKS_API_KEY has been added as a repo secret. The Pullfrog
console model must be set to "Custom (set in workflow)" so the stored
console pick does not override the workflow's model input.
@elviskahoro

Copy link
Copy Markdown
Collaborator Author

Wrong repo — this was meant for the fork. Apologies.

@elviskahoro elviskahoro closed this Oct 3, 2026
elviskahoro added a commit that referenced this pull request Oct 25, 2026
<!-- entire-trail-link-start -->
https://entire.io/gh/elviskahoro/buridan-ui/trails/5
<!-- entire-trail-link-end -->

## What

`pullfrog-approval` (Pullfrog's AI review verdict) is now a **required
check in both enforcement layers**:

- **GitHub side** — ruleset `main - PR requirements` plus classic branch
protection on `main`, both pinned to the Pullfrog app (id 1768019).
`enforce_admins` and the strict up-to-date policy are unchanged. This
blocks direct merges and gates **Trunk merge queue admission** (the
queue only admits PRs GitHub considers mergeable).
- **Trunk queue side** — added to `.trunk/trunk.yaml`
`merge.required_statuses`, so the queue also waits for a **fresh
Pullfrog verdict on the exact merge state it tests**, not just the PR
head at admission.

## Why it is safe to require during queue testing

The queue tests in **draft-PR mode**: trunk-io opens a draft PR from the
`trunk-merge/**` branch, and Pullfrog auto-reviews draft PRs
(`review.drafts` on; the trunk-io app is not in Pullfrog's bot-exclusion
list, which covers
Dependabot/Renovate/Greenkeeper/Snyk/`github-actions[bot]`/changesets).
So `pullfrog-approval` posts on the tested head and the queue
requirement is satisfiable.

The comment in `trunk.yaml` and the paragraph in `AGENTS.md` document
the load-bearing assumptions: if a queue entry ever stalls on a missing
`pullfrog-approval`, check (1) the queue is still in draft-PR mode, (2)
Pullfrog is still reviewing queue draft PRs, (3) the Pullfrog run itself
didn't fail or get cancelled — a failed run posts no check.

## Effects to know

- New PRs: Pullfrog auto-reviews on open and re-reviews on push, so the
check lands on every head automatically. Unresolved Pullfrog feedback
fails the check and blocks the merge — that is the point.
- Queue submissions: the queue now waits for Pullfrog's verdict on the
tested merge state before landing, adding one LLM review (typically a
few minutes, PR-size dependent) per queue test.
- Existing open PRs whose heads were last reviewed before the
approval-check toggle was enabled (e.g. #119, #118, #114, #113, #112,
#108, #107) show `pullfrog-approval` as *Expected — waiting for status*.
Unblock with a one-off `@pullfrog review` comment or a new push.
- If a Pullfrog run fails or is cancelled, no check posts on that head
and the merge/queue stays blocked until the next run reports (documented
Pullfrog behavior).
- A negative verdict on the tested merge state fails the check and
ejects the PR from the queue — address the feedback, push to the
original PR, and resubmit; the new submission opens a new draft PR,
which gets a fresh verdict. No human override exists short of editing
the config.

## Verification

- The GitHub-side gate is verified live on this PR: #126 sat `BLOCKED`
with all CI green while Pullfrog's review was running, cleared once
`pullfrog-approval: SUCCESS` posted, and blocked again on the new
merge-commit head until re-review — exactly the intended per-commit
gate. PR #125 also merged (13:56Z, after the rule went live) with
`pullfrog-approval: SUCCESS` among its checks.
- The queue-side requirement is verified by this PR's own queue run: the
queue reads `required_statuses` from `main`'s trunk.yaml, so #126 is
tested under the old three-check list while Pullfrog independently
auto-reviews the queue's draft PR. During Testing, `gh pr checks
<draft-pr>` will show whether `pullfrog-approval` posts on the
`trunk-merge/**` head. If it does not, the queue entry is cancelled
before landing and the `required_statuses` addition is reverted; if it
does, that evidence is the confirmation the new config is safe for
future submissions.
elviskahoro pushed a commit that referenced this pull request Oct 25, 2026
**Detail bug report:** [View on
Detail](https://app.detail.dev/org_d38a878f-9df9-4c4b-a96a-f5180b860085/bugs/bug_8b90eeca-66ae-45f0-89cd-3a98e14753fa)

Closes #116

## Bug
`TooltipTrigger.create(render_=..., unstyled=<Var[bool]>)` raised an
unrecoverable `VarTypeError` at component construction (`reflex
run`/`reflex export` compile time). The regression was introduced in
#66, which set `TooltipTrigger._merge_default_classes_with_render =
True` to forward the trigger's default centering classes onto custom
`render_` targets. That flag removed the `render_` early-return shield
in `CoreComponent.set_class_name`, so a `Var[bool]` passed as `unstyled`
reached the pre-existing `if props.pop("unstyled", False):` line and was
evaluated via Python `bool(<Var>)`, which Reflex forbids.

The same `if <Var>:` truthiness defect also crashed the non-`render_`
path (`TooltipTrigger.create(unstyled=<Var>)`) before #66 — the
underlying defect is pre-existing and affects every `CoreComponent`. #66
simply turned a tolerated no-op into a crash for the `render_` + `Var`
combination.

## Fix
Rewrote `CoreComponent.set_class_name` (`components/ui/core.py`) so it
never evaluates a Reflex `Var` in a Python boolean context:
- Pops `unstyled` once (default `None`) instead of `pop(..., False)`
inside `if`.
- Handles the documented literal-`True` escape hatch via an identity
check (`unstyled is True`), preserving the existing behavior covered by
#66's tests.
- Routes dynamic `Var[bool]` values through `rx.cond(unstyled,
props_class_name, merged)` so the escape hatch is honored at React
render time instead of crashing at compile time.

This fixes both the #66 regression (`render_` + `Var`) and the
pre-existing `Var`-without-`render_` route with one change. Sibling
components that still rely on the `render_` early-return shield
(`TooltipPopup`, `AccordionTrigger`, `DialogClose`, `CollapsiblePanel`,
`CollapsibleTrigger`) are unaffected —
`_merge_default_classes_with_render` stays `False` for them.

## Testing
- **Unit tests (committed):** added 5 regression tests to
`tests/test_tooltip.py` covering: `render_` + `Var` no-crash;
no-`render_` + `Var` no-crash; the dynamic toggle emits an `rx.cond`
ternary with the caller's class in both branches and the defaults in the
false branch; the no-`render_` + custom-class + `Var` toggle preserves
the caller's class; and the still-shielded `TooltipPopup` + `render_` +
`Var` path tolerates the state var. All 9 tooltip tests pass; the full
suite remains green (829 passed).
- **Routine checks:** pyright reports 0 errors on the touched files;
`ruff format --check` is clean; `ruff check` surfaces only a
pre-existing `RUF012` on `BaseUIComponent.lib_dependencies` (line 58,
present on clean HEAD — out of scope); docs/assets generation produces
no new drift attributable to the fix.
- **End-to-end (not versioned):** registered a temporary page exercising
a state-driven dynamic `unstyled` tooltip, the literal-`True` case, and
the defaults case. With the fix reverted, `reflex compile` crashed with
the exact `VarTypeError` from the report; with the fix, it compiled
successfully. A Playwright run against the dev server confirmed the
trigger's `className` switched from `inline-flex items-center
justify-center` (merged defaults) to `""` (unstyled) when the state flag
was toggled in the browser, confirming the `rx.cond` ternary is reactive
at runtime. The temporary page and a one-off `enable_state=True` flip
needed to mount state at runtime were reverted; the final diff is
isolated to the fix and its tests.

---
_Automatic Fixes PRs can be [configured
here](https://app.detail.dev/org_d38a878f-9df9-4c4b-a96a-f5180b860085/settings/repos/repo_04b9d58c-daa0-46b6-9560-7f39569ec6eb/bugs)._

---------

Co-authored-by: detail-app[bot] <180357370+detail-app[bot]@users.noreply.github.com>
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