Repository navigation
fix(ci): restore avatar browser tests and stabilize fuel exhaustion fixture - #290
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Wasm avatar decoder crates/gui/src/session/avatar/mod.rs, crates/gui/src/session/avatar/web.rs, crates/gui/src/session/avatar/web_platform.rs, docs/measurements/ci-288-289-triage.md |
Wasm builds now use web_platform, which re-exports web and provides the conditional avatar decoder. The triage document records the relocation and browser-test results. |
Idempotent avatar instrumentation .github/workflows/avatar-restore.yml, docs/measurements/ci-288-289-triage.md |
The workflow script adds its probe module and test-only counter instrumentation only when those declarations or increments are absent. The triage document records the change and its validation. |
Plugin-host fuel tests
| Layer / File(s) | Summary |
|---|---|
Fuel-metering test coverage crates/plugin-host/src/tests.rs, docs/measurements/ci-288-289-triage.md |
The spins fixture adds constant/drop pairs to its loop. A separate test checks that a bare branch loop traps with OutOfFuel within the deadline. The triage document records the timeout context and test results. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Possibly related PRs
- oxidezap/client#282: Added the decoder and avatar-restore workflow that this PR updates.
- oxidezap/client#261: Added daemon browser tests that import the avatar web storage implementation.
Suggested reviewers: greptile-apps
Merge Risk: 
71130
-
The reviewed CI fixes preserve their intended behavior, with no identified merge-blocking risk.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely identifies the two primary changes: restoring avatar browser tests and stabilizing the fuel exhaustion fixture. Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 3 files. (2 skipped: 2 unsupported.)
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
|
@codex review Please review final head a1758d0. Check that the GUI web_platform adapter preserves the same decode/storage entry points while daemon tests still compile and exercise the actual web.rs storage implementation. Also check the fuel fixture: deadline, unconditional loop, stopped-state/fuel assertions and 50,000,000-unit production budget remain unchanged; const/drop pairs consume Wasmi 2 operator fuel before folding. Baseline/main reproduced both failures; final browser suites and native plugin-host tests passed. Checkpoint: docs/measurements/ci-288-289-triage.md. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review Please review final head 0e3e4b8. It adds an explicit bounded bare-branch probe so the cheaper integration fixture cannot hide a free-branch metering regression. The original empty Final local validation: 136 plugin-host tests passed, one existing ignored; Clippy all targets/features with warnings denied and formatting passed. Final browser layout passed all 16 daemon + 32 session tests. Checkpoint updated in docs/measurements/ci-288-289-triage.md. Previous a1758d0 review completed without findings; this request covers the additional bare-branch coverage. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review Please review final head 71130e9. The Rust source is unchanged from reviewed 0e3e4b8. The avatar comparison workflow failed in its BASE measurement with E0428: it unconditionally reinjected Executed the actual Python block extracted from YAML on current main and an uninstrumented legacy fixture. Each produced exactly one probe/counter/increment; a second application was byte-identical. Main input was entirely byte-identical after instrumentation. Detailed checkpoint appended to docs/measurements/ci-288-289-triage.md. On the previous head, workspace Clippy, workspace tests, doctests, CLI budget and host example checks had passed; final-head CI reruns after this workflow-only fix. |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Three baseline CI problems blocked the profiling PRs #288 and #289. This PR separates GUI avatar decoding from the actual storage file used by daemon browser tests, makes the infinite-loop fuel fixture cheaper while retaining explicit bare-branch coverage, and prevents duplicate avatar probe instrumentation in the comparison workflow. Storage behavior, the production fuel budget, deadlines, assertions and measurement coverage remain intact.
Baseline failures, reproduction and classifications are recorded in docs/measurements/ci-288-289-triage.md. Validation passed: 136 native plugin-host tests, 16 daemon and 32 session browser tests, Clippy with warnings denied and formatting. Workflow instrumentation was verified on current-main and legacy inputs, including repeated application.
Final head 71130e9 is qualified: CI, Pages, Avatar restore and Release artifacts all completed successfully. Codex reviewed this exact head without findings; no unresolved review threads or merge conflicts. Ready for user review and merge. No PR merge was performed.
Summary by CodeRabbit