Skip to content

ui: problemconfig: guard empty config load for js-yaml v5 - #1208

Merged
undefined-moe merged 1 commit into
hydro-dev:masterfrom
gtn1024:fix-testcase-config
Sep 11, 2026
Merged

ui: problemconfig: guard empty config load for js-yaml v5#1208
undefined-moe merged 1 commit into
hydro-dev:masterfrom
gtn1024:fix-testcase-config

Conversation

@gtn1024

@gtn1024 gtn1024 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes
    • Improved configuration loading when no configuration data is provided, preventing errors and allowing an empty configuration to load successfully.

js-yaml v5 throws on empty input where v4 returned undefined, so a
problem without config.yaml landed in the error branch (__valid=false):
config page forced to the Errors tab, subtask auto-inference invisible,
and the editor never synced. Default to '{}' like the server-side fix
(87f64d6) so empty configs load as an empty subtask list.
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 30a2042a-14e6-4268-bee6-6ad581bcc2e9

📥 Commits

Reviewing files that changed from the base of the PR and between 67b6053 and 1b1cf3a.

📒 Files selected for processing (1)
  • packages/ui-default/components/problemconfig/reducer/config.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


Walkthrough

The CONFIG_LOAD_FULFILLED reducer case now passes '{}' to yaml.load when action.payload.config is falsy or missing. Existing configuration values continue to pass directly to the parser.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix

Suggested reviewers: undefined-moe

Merge Risk: ⚪ Minimal · up to 1b1cf

The change safely handles missing problem configuration without introducing a material merge risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the UI problem configuration change and the guard for empty config loading with js-yaml v5.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/ui-default/components/problemconfig/reducer/config.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.


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.

@undefined-moe
undefined-moe merged commit 6635257 into hydro-dev:master Sep 11, 2026
6 checks passed
@gtn1024
gtn1024 deleted the fix-testcase-config branch September 11, 2026 14:02
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.

2 participants