Skip to content

Conversation

@tuandao-zenai
Copy link

@tuandao-zenai tuandao-zenai commented Sep 17, 2025

Summary by CodeRabbit

  • Chores
    • Improved CLI validation logging: spot-check summary lines now include the wallet UID, making it easier to correlate results to specific wallets when reviewing logs. No changes to validation behavior, sampling, or outputs; this update only augments log context. This is useful for debugging and audits in multi-wallet environments and requires no configuration changes.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 17, 2025

Walkthrough

Adds per-wallet context to an existing spot-check log line in _process_wallet_window within grail/cli/validate.py by appending the originating uid string. No control flow, sampling, grouping, or validation logic changes.

Changes

Cohort / File(s) Change Summary
CLI validation logging
grail/cli/validate.py
Augmented a log message in _process_wallet_window to include "from uid {uid_str}" alongside rollout/group counts; no functional changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

I thump my logs with careful cheer,
A uid whisper, crisp and clear.
No hops in flow, no twisting trail—
Just context added to the tale.
Ears up high, I note with pride:
More breadcrumbs for the validation ride. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title matches the main change: the PR updates the spot-check logging to indicate which uid produced the check by appending "from uid {uid_str}", so it accurately summarizes the primary change even though the wording is slightly awkward.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
grail/cli/validate.py (1)

1096-1101: Fix logged percentage to reflect actual sampled groups, not the target SAMPLE_RATE.

The current log always prints e.g. “10% of groups” even when rounding forces sampling 1 group (which could be >10%). Log the realized ratio for accuracy.

Apply this minimal change:

         logger.info(
-            f"📊 Spot checking {len(indices_to_check)}/{total_inferences} "
-            f"rollouts from {groups_to_check}/{num_groups} groups "
-            f"({SAMPLE_RATE * 100:.0f}% of groups) "
-            f"from uid {uid_str}"
+            f"📊 Spot checking {len(indices_to_check)}/{total_inferences} "
+            f"rollouts from {groups_to_check}/{num_groups} groups "
+            f"({(groups_to_check / num_groups * 100.0) if num_groups else 0.0:.0f}% of groups) "
+            f"from uid {uid_str}"
         )

Optional: switch to parameterized logging to avoid eager f-string formatting on disabled levels:

-        logger.info(
-            f"📊 Spot checking {len(indices_to_check)}/{total_inferences} "
-            f"rollouts from {groups_to_check}/{num_groups} groups "
-            f"({(groups_to_check / num_groups * 100.0) if num_groups else 0.0:.0f}% of groups) "
-            f"from uid {uid_str}"
-        )
+        actual_pct = (groups_to_check / num_groups * 100.0) if num_groups else 0.0
+        logger.info(
+            "📊 Spot checking %d/%d rollouts from %d/%d groups (%.0f%% of groups) from uid %s",
+            len(indices_to_check), total_inferences, groups_to_check, num_groups, actual_pct, uid_str,
+        )
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d9f6d5a and 05255f8.

📒 Files selected for processing (1)
  • grail/cli/validate.py (1 hunks)
🔇 Additional comments (1)
grail/cli/validate.py (1)

1096-1101: Nice: added uid context improves traceability.

Appending “from uid {uid_str}” makes spot-check logs actionable when multiple wallets are processed in the same window.

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