Skip to content

fix(fallback): reject null manual rules and cover responses guards#184

Merged
SantiagoDePolonia merged 1 commit intomainfrom
fix/fallback-followups
Mar 28, 2026
Merged

fix(fallback): reject null manual rules and cover responses guards#184
SantiagoDePolonia merged 1 commit intomainfrom
fix/fallback-followups

Conversation

@SantiagoDePolonia
Copy link
Copy Markdown
Contributor

@SantiagoDePolonia SantiagoDePolonia commented Mar 28, 2026 •
edited by coderabbitai bot
Loading

Summary

  • reject null values in fallback manual rules instead of silently treating them as empty chains
  • add config coverage for null manual rule entries
  • add negative Responses fallback tests for non-availability errors and execution-policy-disabled fallback

Verification

  • go test ./config ./internal/server
  • go test ./...

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Configuration validation now enforces stricter rules for fallback manual configuration, explicitly rejecting null values to prevent invalid settings from being loaded.

  • Tests

    • Added test coverage for configuration validation with null value handling. Added tests verifying fallback behavior correctly does not trigger under non-availability errors or when features are disabled.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 28, 2026 •
edited
Loading

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 789474a0-c128-40ed-8b8f-3a162535c0d0

📥 Commits

Reviewing files that changed from the base of the PR and between 180fc9d and 54d2fc9.

📒 Files selected for processing (3)
  • config/config.go
  • config/config_test.go
  • internal/server/fallback_test.go
📝 Walkthrough

Walkthrough

The PR enhances JSON parsing in fallback configuration to explicitly reject null values for manual rules paths and adds comprehensive tests validating null rejection and conditions where fallback is not triggered.

Changes

Cohort / File(s) Summary
Config Validation Enhancement
config/config.go, config/config_test.go		 Enhanced JSON parsing in loadFallbackConfig to decode values as intermediate json.RawMessage and validate against explicit null values, rejecting them with descriptive errors. Added test TestLoad_FallbackManualRulesRejectsNullValues to verify null rejection for model keys.
Fallback Streaming Behavior Tests
internal/server/fallback_test.go		 Added two new Responses streaming tests verifying fallback is not triggered: one for non-availability errors (400 BadRequest, primary model only), another for availability errors with ExecutionFeatures.Fallback: false (503 Service Unavailable, primary model only).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Poem

🐰 A rabbit hops through JSON trees,
Casting out the nulls with ease,
No more silent, empty air—
Validation's gentle, watchful care! ✨

✨ 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 fix/fallback-followups

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.

@SantiagoDePolonia SantiagoDePolonia marked this pull request as ready for review March 28, 2026 18:39
@SantiagoDePolonia SantiagoDePolonia merged commit 6b4608e into main Mar 28, 2026
15 checks passed
@SantiagoDePolonia SantiagoDePolonia deleted the fix/fallback-followups branch April 1, 2026 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@SantiagoDePolonia