Conversation
|
Warning Review limit reached
More reviews will be available in 58 minutes and 42 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds two service interface enhancements: updated imports in ChallanSearch for service type support, and a new async CRC verification dispatcher in Verification that delegates to the Idfy provider with fallback error handling. ChangesExternal Service Interface Enhancements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/mobility-core/src/Kernel/External/ChallanSearch/Interface.hs (1)
20-25: 🏗️ Heavy liftConsider
NonEmptyfor compile-time safety.The function accepts
[ChallanSearchService]and validates non-empty at runtime (line 27). UsingNonEmpty ChallanSearchServicewould enforce the non-empty constraint at the type level, eliminating the runtime check and preventing invalid calls at compile time.♻️ Proposed signature with NonEmpty
+import Data.List.NonEmpty (NonEmpty) + getPendingChallanCountWithFallback :: (EncFlow m r, CoreMetrics m, MonadFlow m, Log m, HasRequestId r, MonadReader r m) => (ChallanSearchService -> m ChallanSearchServiceConfig) -> - [ChallanSearchService] -> + NonEmpty ChallanSearchService -> PendingChallanReq -> m PendingChallanRespThen remove the runtime check at line 27.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/External/ChallanSearch/Interface.hs` around lines 20 - 25, Change the signature of getPendingChallanCountWithFallback to accept NonEmpty ChallanSearchService instead of [ChallanSearchService] (import Data.List.NonEmpty or qualified NonEmpty) and update all call sites accordingly so callers must provide a non-empty list; then remove the runtime non-empty validation inside getPendingChallanCountWithFallback (the check at line 27) because the NonEmpty type guarantees at-compile-time that at least one ChallanSearchService exists. Ensure any pattern matches or folds over the services use NonEmpty constructors/functions (e.g., toList, head, NE.nonEmpty helpers) as needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/mobility-core/src/Kernel/External/ChallanSearch/Interface.hs`:
- Line 21: The signature for getPendingChallanCountWithFallback includes a
redundant MonadFlow m constraint because EncFlow m r (i.e., HasFlowEnv) already
implies MonadFlow (and thus TryException) which withTryCatch needs; remove
MonadFlow m from the constraint list and keep EncFlow m r, CoreMetrics m, Log m,
HasRequestId r, MonadReader r m (and any required TryException bound is already
satisfied via EncFlow/HasFlowEnv).
---
Nitpick comments:
In `@lib/mobility-core/src/Kernel/External/ChallanSearch/Interface.hs`:
- Around line 20-25: Change the signature of getPendingChallanCountWithFallback
to accept NonEmpty ChallanSearchService instead of [ChallanSearchService]
(import Data.List.NonEmpty or qualified NonEmpty) and update all call sites
accordingly so callers must provide a non-empty list; then remove the runtime
non-empty validation inside getPendingChallanCountWithFallback (the check at
line 27) because the NonEmpty type guarantees at-compile-time that at least one
ChallanSearchService exists. Ensure any pattern matches or folds over the
services use NonEmpty constructors/functions (e.g., toList, head, NE.nonEmpty
helpers) as needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29808e32-450d-4dea-8b26-0b4c648eb998
📒 Files selected for processing (2)
lib/mobility-core/src/Kernel/External/ChallanSearch/Interface.hslib/mobility-core/src/Kernel/External/Verification/Interface.hs
| SignzyChallanSearch cfg -> CIS.signzyChallanSearch cfg req | ||
|
|
||
| getPendingChallanCountWithFallback :: | ||
| (EncFlow m r, CoreMetrics m, MonadFlow m, Log m, HasRequestId r, MonadReader r m) => |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find the type signature of withTryCatch and TryException class definition
# Search for withTryCatch type signature
rg -n -A 2 'withTryCatch\s*::' --type=hs
# Search for TryException typeclass definition
rg -n -A 5 'class.*TryException' --type=hsRepository: nammayatri/shared-kernel
Length of output: 734
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the function and where withTryCatch is used in Interface.hs
sed -n '1,120p' lib/mobility-core/src/Kernel/External/ChallanSearch/Interface.hs | cat -n
# Show the full TryException definition
sed -n '1,120p' lib/mobility-core/src/Kernel/Types/TryException.hs | cat -n
# Locate MonadFlow definition and inspect its constraints
rg -n --type=hs "class\s+MonadFlow\b|type\s+MonadFlow\b|MonadFlow\s+m" lib/mobility-core/src/Kernel lib/mobility-core/src | head -n 50
# If a file is found, print likely definition area(s)
# (Fallback: directly search likely patterns)
file="$(rg -n --type=hs -l "class\s+MonadFlow\b" lib/mobility-core/src/Kernel lib/mobility-core/src | head -n 1 || true)"
if [ -n "${file}" ]; then
echo "---- MonadFlow file: ${file} ----"
sed -n '1,160p' "$file" | cat -n
fiRepository: nammayatri/shared-kernel
Length of output: 8289
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --type=hs "class\s+MonadFlow\b|type\s+MonadFlow\b" lib/mobility-core/src/Kernel | head -n 50
# Also search in other likely dirs for the MonadFlow definition
rg -n --type=hs "class\s+MonadFlow\b|type\s+MonadFlow\b" lib/mobility-core/src | head -n 50
# Find where TryException is used with MonadFlow (maybe MonadFlow extends it)
rg -n --type=hs "TryException\s+m|TryException\b" lib/mobility-core/src/Kernel/Types lib/mobility-core/src/Kernel | head -n 80Repository: nammayatri/shared-kernel
Length of output: 8786
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show MonadFlow definition in Kernel/Types/App.hs (around the referenced line)
sed -n '1,120p' lib/mobility-core/src/Kernel/Types/App.hs | cat -n
# Show TryException instance for FlowR in Kernel/Types/Flow.hs (around the referenced line)
sed -n '240,320p' lib/mobility-core/src/Kernel/Types/Flow.hs | cat -n
# Show imports/instances nearby in TryException for completeness
sed -n '1,120p' lib/mobility-core/src/Kernel/Types/TryException.hs | cat -n
# Show full relevant signature context in Interface.hs (around withTryCatch usage)
sed -n '1,90p' lib/mobility-core/src/Kernel/External/ChallanSearch/Interface.hs | cat -nRepository: nammayatri/shared-kernel
Length of output: 10212
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find EncFlow definition
enc_file="$(rg -n --type=hs "type\s+EncFlow|class\s+EncFlow" lib/mobility-core/src/Kernel | head -n 1 | cut -d: -f1 || true)"
echo "EncFlow file: ${enc_file:-<not found>}"
if [ -n "${enc_file:-}" ]; then
sed -n '1,200p' "$enc_file" | cat -n
fi
# Inspect Kernel.Types.Common for relevant constraints (if EncFlow not found above)
if [ -z "${enc_file:-}" ]; then
rg -n --type=hs "EncFlow\s+m" lib/mobility-core/src/Kernel/Types lib/mobility-core/src/Kernel | head -n 50
fi
# Inspect Kernel.Prelude reexports for withTryCatch/logDebug/throwError if available
rg -n --type=hs "withTryCatch|logDebug|throwError" lib/mobility-core/src/Kernel/Prelude lib/mobility-core/src/Kernel | head -n 80Repository: nammayatri/shared-kernel
Length of output: 18190
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --type=hs "module Kernel\.Types\.Common|EncFlow\s" lib/mobility-core/src/Kernel/Types/Common.hs
sed -n '1,140p' lib/mobility-core/src/Kernel/Types/Common.hs | cat -n
# Check whether Interface.hs brings EncFlow from Kernel.Types.Common or another import
rg -n --type=hs "EncFlow" lib/mobility-core/src/Kernel/External/ChallanSearch/Interface.hsRepository: nammayatri/shared-kernel
Length of output: 6084
MonadFlow m constraint is redundant in getPendingChallanCountWithFallback.
EncFlow m r expands to HasFlowEnv m r ..., and HasFlowEnv already requires MonadFlow m (which includes TryException m). Since withTryCatch needs TryException m, you can drop MonadFlow m from the signature.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/mobility-core/src/Kernel/External/ChallanSearch/Interface.hs` at line 21,
The signature for getPendingChallanCountWithFallback includes a redundant
MonadFlow m constraint because EncFlow m r (i.e., HasFlowEnv) already implies
MonadFlow (and thus TryException) which withTryCatch needs; remove MonadFlow m
from the constraint list and keep EncFlow m r, CoreMetrics m, Log m,
HasRequestId r, MonadReader r m (and any required TryException bound is already
satisfied via EncFlow/HasFlowEnv).
3 participants
Type of Change
Description
Crc/enh
Additional Changes
Motivation and Context
How did you test it?
Checklist
./dev/format-all-files.shSummary by CodeRabbit