Skip to content

fix: CodeRabbit follow-ups from #144 + #141 (incl. critical /api/v1 double-prefix)#152

Merged
Gradata merged 1 commit intomainfrom
fix/coderabbit-followups
May 1, 2026
Merged

fix: CodeRabbit follow-ups from #144 + #141 (incl. critical /api/v1 double-prefix)#152
Gradata merged 1 commit intomainfrom
fix/coderabbit-followups

Conversation

@Gradata
Copy link
Copy Markdown
Owner

@Gradata Gradata commented May 1, 2026

Applies 11 CR action items. Critical: cloud_sync double /api/v1 prefix broke telemetry + corpus POSTs in prod.

Critical:
- cloud/sync.py: fix double /api/v1 prefix on telemetry + corpus paths

Major:
- cli.py: resolve brain_root once for skill export consistency
- skill_export.py: escape backslashes in YAML descriptions
- skill_export.py: whitespace-only desc falls back to auto
- implicit_feedback.py: negative signals win over approval on conflict
- inject_brain_rules.py: harden MAX_RULES int parse against malformed env

Tests:
- update assertions for corrected /telemetry + /corpus paths
- add regression coverage for YAML backslash/newline/whitespace
- tighten loose assertions in hooks_intelligence
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 1, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6a255861-6270-4df4-805c-9e1bed621cb2

📥 Commits

Reviewing files that changed from the base of the PR and between fbf8dbb and 78f527b.

📒 Files selected for processing (8)
  • Gradata/src/gradata/cli.py
  • Gradata/src/gradata/cloud/sync.py
  • Gradata/src/gradata/enhancements/skill_export.py
  • Gradata/src/gradata/hooks/implicit_feedback.py
  • Gradata/src/gradata/hooks/inject_brain_rules.py
  • Gradata/tests/test_cloud_sync.py
  • Gradata/tests/test_hooks_intelligence.py
  • Gradata/tests/test_skill_export.py

📝 Walkthrough

Summary

  • Critical fix: Removed double /api/v1 prefix in cloud/sync.py POST paths for telemetry and corpus endpoints, correcting routing that was broken in production
  • Improved brain root resolution in skill export by resolving brain_root once before creating Brain instance instead of falling back to CWD
  • Enhanced YAML frontmatter generation with backslash escaping for valid Windows-style paths and literal sequences
  • Refined description handling in skill export to treat whitespace-only descriptions as auto-generation triggers (falling back to synthesized descriptions)
  • Modified implicit feedback hook to make negative feedback signals override approval patterns when both present in same message
  • Hardened GRADATA_WISDOM_MAX_RULES environment variable parsing to emit warning and fall back to default (9) instead of crashing on invalid values
  • Updated cloud sync and hooks intelligence tests to match corrected endpoint paths and tightened assertion specificity
  • Added regression test coverage for YAML backslash/newline/whitespace handling in skill export

Walkthrough

The PR addresses multiple bug fixes and logic improvements: correcting Brain instantiation in the CLI export command, updating cloud sync endpoint paths to rely on pre-configured API base URLs, enhancing YAML description escaping for Windows paths, filtering conflicting feedback signals in hook processing, and adding environment variable validation to prevent runtime errors.

Changes

Cohort / File(s) Summary
Skill Export Logic
src/gradata/cli.py, src/gradata/enhancements/skill_export.py, tests/test_skill_export.py
Updated cmd_skill_export to instantiate Brain locally from resolved brain_root instead of calling _get_brain(). Enhanced YAML frontmatter escaping of backslashes before quotes for description fields. Extended test coverage with 4 new cases validating backslash escaping and whitespace fallback behavior.
Cloud Sync Endpoints
src/gradata/cloud/sync.py, tests/test_cloud_sync.py
Removed hard-coded /api/v1 prefix from POST request paths for telemetry and corpus endpoints, relying on pre-configured config.api_base for versioning. Updated corresponding test assertions to expect unversioned paths.
Implicit Feedback Hook
src/gradata/hooks/implicit_feedback.py, tests/test_hooks_intelligence.py
Added logic to suppress approval signals when negative feedback signals are detected, preventing conflicting OUTPUT_ACCEPTED emissions. Tightened test assertions to validate exact output payloads rather than substring matching.
Brain Rules Environment Validation
src/gradata/hooks/inject_brain_rules.py
Added explicit validation for GRADATA_WISDOM_MAX_RULES environment variable with warning fallback instead of unconditional type casting, preventing runtime errors on unparseable values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

bug

✨ 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/coderabbit-followups

Review rate limit: 3/5 reviews remaining, refill in 19 minutes and 40 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@Gradata Gradata merged commit 14de072 into main May 1, 2026
8 of 9 checks passed
@Gradata Gradata deleted the fix/coderabbit-followups branch May 1, 2026 15:54
@coderabbitai coderabbitai Bot added the bug Something isn't working label May 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant