Skip to content

fix: make .coderabbit.yaml valid so the repo config is actually used - #34

Merged
wei-hai merged 1 commit into
mainfrom
fix/coderabbit-config-schema
Jul 30, 2026
Merged

fix: make .coderabbit.yaml valid so the repo config is actually used#34
wei-hai merged 1 commit into
mainfrom
fix/coderabbit-config-schema

Conversation

@wei-hai

@wei-hai wei-hai commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

The bug

tone_instructions is capped at 250 characters by the CodeRabbit schema. This file held 539, so the entire config failed validation — and CodeRabbit silently fell back to org-level UI settings rather than erroring. Evidence: recent reviews on this repo report

Configuration used: Organization UI

So none of the reviewer guidance in this file has ever taken effect.

I verified this rather than taking the bot's word for it — fetched https://coderabbit.ai/integrations/schema.v2.json and validated the file with jsonschema:

tone_instructions                         maxLength:   250
reviews.path_instructions[].instructions  maxLength: 20000
before: 1 schema error  (tone_instructions: 539 > 250)
after:  0 schema errors (tone_instructions: 136)

That single field was the only error in the file.

The fix

The schema describes tone_instructions as a persona field — "Set the tone of reviews and chat. Example: 'You must talk like Mr. T.'" — not a place for substantive review rules. So rather than deleting ~290 characters of guidance to squeeze under the cap, the repo-wide rules move to a path: "**" entry, which has 20000 characters of room.

No guidance is lost:

Old tone_instructions clause Where it lives now
Don't repeat ruff/mypy findings, don't nitpick style new ** path_instruction
Only flag issues this PR introduces, not pre-existing code new ** path_instruction
Public API stability / breaking signature changes already in src/comfy_sdk/** (unchanged)
Correct error handling for network calls already in src/comfy_sdk/** (unchanged)
Accurate, exported type hints already in src/comfy_sdk/** (unchanged)

The last three were already duplicated verbatim in path_instructions, so moving only the two genuinely cross-cutting rules keeps the diff minimal.

tone_instructions now carries actual tone direction, which is what the field is for.

How to confirm it worked

The next CodeRabbit review on this repo should stop saying Configuration used: Organization UI. This PR is itself the first test.

The sibling fix for comfy-typescript-sdk is in that repo's PR.

tone_instructions is capped at 250 characters by the CodeRabbit schema and
this file held 539, so the whole config failed validation and reviews silently
fell back to the org-level UI settings — none of the guidance here has been
taking effect.

The schema describes tone_instructions as a persona field ("Set the tone of
reviews and chat"), not a place for review rules, and gives
path_instructions 20000 characters. So the two repo-wide rules move to a
`**` path_instruction rather than being cut: don't repeat what ruff/mypy
already report, and don't flag pre-existing issues in moved or reformatted
code.

Nothing is lost. The rest of the old tone_instructions — public API
stability, network error handling, type hints on the public surface — was
already covered verbatim by the src/comfy_sdk/** entry.

Validated against https://coderabbit.ai/integrations/schema.v2.json: 1 error
before, 0 after.
@wei-hai
wei-hai requested review from a team as code owners July 30, 2026 20:56
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories.

Warning

.coderabbit.yaml has a parsing error

The CodeRabbit configuration file in this repository has a parsing error and default settings were used instead. Please fix the error(s) in the configuration file. You can initialize chat with CodeRabbit to get help with the configuration file.

💥 Parsing errors (1)
Validation error: Too big: expected string to have <=250 characters at "tone_instructions"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c39f3277-84e5-4be5-91ef-9822618a772a

📥 Commits

Reviewing files that changed from the base of the PR and between 8688e9e and 1759f9f.

📒 Files selected for processing (1)
  • .coderabbit.yaml

📝 Walkthrough

Walkthrough

.coderabbit.yaml shortens tone instructions and moves detailed review rules into repo-wide path_instructions, including guidance for linting, typing, introduced issues, and reformatted code.

Changes

Review guidance configuration

Layer / File(s) Summary
Tone and path instruction placement
.coderabbit.yaml
tone_instructions is reduced to concise directives, while detailed review rules are placed under the "**" path filter due to the schema character limit.

Suggested reviewers: alexisrolland

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/coderabbit-config-schema
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/coderabbit-config-schema

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.

@coderabbitai
coderabbitai Bot requested a review from alexisrolland July 30, 2026 20:57
@wei-hai
wei-hai merged commit fa866b8 into main Jul 30, 2026
10 checks passed
@wei-hai
wei-hai deleted the fix/coderabbit-config-schema branch July 30, 2026 20:58
@wei-hai

wei-hai commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Correction + stronger evidence.

I claimed in the description that this PR would be its own test — that CodeRabbit's review here should stop saying Configuration used: Organization UI. That was wrong, and it did not happen: this PR's review still reports Organization UI.

The reason is that CodeRabbit reads .coderabbit.yaml from the PR's base branch, not the head. main still has the oversized value, so the fix cannot show up until this merges. A config-file PR can't validate itself.

What I could do instead is test the hypothesis across repos. Comparing tone_instructions length against the config source CodeRabbit reports:

Repo tone_instructions Configuration used
cloud 210 (≤250) Path: .coderabbit.yaml
comfy-api-proxy 478 Organization UI
comfy-python-sdk (main) 539 Organization UI
comfy-typescript-sdk (main) 509 Organization UI

The correlation is exact, and cloud proves repo config can win — so this isn't a case of the org UI unconditionally overriding. The 250-char cap is the discriminator.

So the fix stands, but the confirmation has to come from the first CodeRabbit review on main after this merges, not from this PR. Worth a quick check then.

Separately: comfy-api-proxy has the same bug (478 chars) and needs the same fix.

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