Skip to content

Conversation

@salgozino
Copy link
Contributor

@salgozino salgozino commented Aug 28, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Improved robustness in dispute processing with a fallback vote-counting method to handle data retrieval failures.
    • Gracefully handles already-executed disputes and read errors without causing runtime interruptions.
  • Chores

    • Added comprehensive runtime diagnostics, including detailed logs during dispute processing, vote counting, and phase transitions to aid monitoring and troubleshooting.
    • Enhanced debug messages to clarify system readiness when moving between staking, generating, and drawing phases.
    • Post-processing logs now summarize vote counter results and tie/coherence status for clearer operational insights.

@coderabbitai
Copy link

coderabbitai bot commented Aug 28, 2025

Walkthrough

Adds extensive debug logging to dispute processing, introduces a manual vote-counter fallback when contract calls fail, logs post-fetch diagnostics, handles already-executed disputes with explicit logs, and adds readiness logs for phase transitions. No public API changes.

Changes

Cohort / File(s) Summary of changes
Dispute processing diagnostics and fallback
src/bots/kleros-liquid.js
- Added verbose logs for dispute processing, vote-counter resolution, coherence/tie status, and phase readiness
- Replaced catch(_) with catch(e); error logging commented; breaks on error
- Implemented manual vote-counter computation when getVoteCounter fails, with per-vote logging
- Log when disputes are already executed
- Minor typo in a log message (“reay”)

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Scheduler
  participant Bot as kleros-liquid bot
  participant Kleros as KlerosContract

  Scheduler->>Bot: run()
  Note over Bot: "Processing dispute <id>"
  Bot->>Kleros: getDispute(disputeID)
  alt Dispute already executed
    Bot->>Bot: log "Dispute <id> is already executed."
    Bot-->>Scheduler: continue next
  else Not executed
    loop Resolve vote counters
      Bot->>Kleros: getVoteCounter(disputeID, i)
      alt Success
        Bot->>Bot: log counter
      else Failure
        Bot->>Bot: log "Looking up vote counter manually..."
        loop Iterate votes j
          Bot->>Kleros: getVote(disputeID, j)
          Bot->>Bot: log "Vote for dispute <id>, choice <i>, index <j>..."
        end
      end
    end
    Bot->>Bot: log "Vote counter has N entries"
    Bot->>Bot: log coherence/tie status
    Note over Bot: "Checking if reay to move phase"
    alt Ready staking -> generating
      Bot->>Bot: log readiness
    else Ready generating -> drawing
      Bot->>Bot: log readiness
    else Ready drawing -> staking
      Bot->>Bot: log readiness
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Fix: heartbeat & gas Logic #60 — Also modifies src/bots/kleros-liquid.js with logging, error handling, and vote-count logic, overlapping in the same control flow.

Suggested labels

bug

Suggested reviewers

  • jaybuidl
  • kemuru

Poem

Ears up, I hop through disputes with care,
Counting votes when counters aren’t there.
Logs like carrots, crisp and bright,
Phases turn from day to night.
If already done, I twitch and scoot—
Next case awaits this busy boot! 🥕🐇

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 589d81e and 3ee3719.

📒 Files selected for processing (1)
  • src/bots/kleros-liquid.js (6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/bots/kleros-liquid.js
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/add-logs

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbit in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbit in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbit gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbit read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbit help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbit ignore or @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbit summary or @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbit or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • 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

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@salgozino salgozino self-assigned this Aug 28, 2025
Copy link

@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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/bots/kleros-liquid.js (1)

19-21: Heartbeat stays disabled after one error; re-enable at next loop.

doHeartbeat is set to false on error and never reset, suppressing liveness pings forever after a transient failure.

   while (true) {
     console.log("Initializing klerosLiquid loop...");
+    doHeartbeat = true; // reset for this iteration

Optionally improve error visibility:

-    } catch (e) {
-      console.error("Failed to process disputes: ", e);
+    } catch (e) {
+      console.error("Failed to process disputes: %s", (e && (e.stack || e.message)) || e);
       doHeartbeat = false;
     }

Also applies to: 167-169, 210-217

🧹 Nitpick comments (5)
src/bots/kleros-liquid.js (5)

178-178: Typo in log message.

Change “reay” → “ready”.

-    console.log(`Checking if reay to move phase`);
+    console.log(`Checking if ready to move phase`);

173-191: Use strict equality by normalizing phase to a number.

Avoid loose equality and implicit coercions for clarity and safety.

-    const phase = await klerosLiquid.methods.phase().call();
+    const phase = Number(await klerosLiquid.methods.phase().call());

-    if (phase == PhaseEnum.staking) {
+    if (phase === PhaseEnum.staking) {
...
-    } else if (phase == PhaseEnum.generating) {
+    } else if (phase === PhaseEnum.generating) {
...
-    } else if (phase == PhaseEnum.drawing) {
+    } else if (phase === PhaseEnum.drawing) {

104-104: Add dispute ID to vote-counter size log for easier correlation.

-          console.debug(`Vote counter has ${voteCounters.length} entries`);
+          console.debug(`Vote counter for dispute ${disputeID} has ${voteCounters.length} entries`);

112-114: Log array content explicitly.

Template-literal interpolation of arrays prints comma-joined values; use JSON for clarity.

-          console.debug(
-            `No tie and no coherent votes for dispute ${disputeID}: ${notTieAndNoOneCoherent}`
-          );
+          console.debug(
+            `No tie and no coherent votes for dispute ${disputeID}: ${JSON.stringify(notTieAndNoOneCoherent)}`
+          );

37-37: Consider reducing per-ID debug noise or gating via env.

This will emit one line per dispute on every run; consider sampling or enabling only with VERBOSE=true.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7fa4bdb and 589d81e.

📒 Files selected for processing (1)
  • src/bots/kleros-liquid.js (5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/bots/kleros-liquid.js (1)
src/xdai-bots/x-kleros-liquid.js (12)
  • disputeID (46-46)
  • executedDisputeIDs (22-22)
  • dispute (49-51)
  • voteCounters (56-105)
  • notTieAndNoOneCoherent (107-112)
  • phase (170-170)
  • PhaseEnum (19-19)
  • minStakingTime (178-180)
  • lastPhaseChange (171-173)
  • disputesWithoutJurors (174-176)
  • readyForNextPhase (169-169)
  • maxDrawingTime (193-195)
🔇 Additional comments (1)
src/bots/kleros-liquid.js (1)

42-45: Good: surfaced read-dispute failure with context.

Replacing the silent catch with a specific error log (including the dispute ID and message) is a solid operational improvement.

Comment on lines +61 to +63
console.debug(
`Looking up vote counter manually for dispute ${disputeID}, choice ${i}`
);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix fallback path: missing .call() and shape mismatch cause runtime errors.

  • getVote requires .call(); without it, the await resolves to a method object, not data.
  • The manual voteCounter lacks counts, but later logic indexes v.counts[winningChoice], leading to TypeError.

Apply:

-                  const vote = await klerosLiquid.methods.getVote(
-                    disputeID,
-                    i,
-                    j
-                  );
+                  const vote = await klerosLiquid.methods
+                    .getVote(disputeID, i, j)
+                    .call();

And return a compatible structure with counts (stringified), ensuring the winner exists:

-                voteCounter = {
-                  tied,
-                  winningChoice,
-                };
+                const counts = Object.fromEntries(
+                  Object.entries(_voteCounters).map(([k, v]) => [String(k), String(v)])
+                );
+                if (counts[winningChoice] === undefined) counts[winningChoice] = "0";
+                voteCounter = { tied, winningChoice, counts };

Also applies to: 69-73, 95-99, 106-111

🤖 Prompt for AI Agents
In src/bots/kleros-liquid.js around lines 61-63 (and similarly at 69-73, 95-99,
106-111), the fallback path for fetching vote counters is incorrect: it calls
the contract method without .call(), which returns a method object instead of
data, and it returns an object lacking a counts array so later indexing
v.counts[winningChoice] throws. Fix by invoking the contract getter with .call()
and by returning a structure matching the expected shape: an object with counts
as an array of strings (stringify numeric values) and a winner index validated
to exist (if winner not present, set to a safe default or throw/handle
accordingly); ensure the returned counts length matches numberOfChoices to avoid
out-of-bounds indexing.

Copy link

@Parsa-Darbouy Parsa-Darbouy left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@salgozino salgozino merged commit 9cbf39b into master Aug 28, 2025
3 checks passed
@salgozino salgozino deleted the fix/add-logs branch August 28, 2025 16:35
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.

3 participants