Conversation
📝 WalkthroughWalkthroughThe changes refactor team code generation across multiple authentication flows by replacing do-while loops with bounded while loops (10-attempt limit), adding explicit error handling to distinguish between collision detection (retry) and network failures (fail-fast), and ensuring consistent error reporting across createTeam, createTeamWithGoogle, and OTP-based signup verification paths. Changes
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/services/ai_service.dart (1)
380-380:⚠️ Potential issue | 🟡 MinorAvoid logging the full request body (contains API key in URL and potentially sensitive user messages).
Line 380 logs the entire
requestBody(which includes chat history with user messages) and line 527 does the same. In production, this could leak PII or sensitive conversation content into logs. Consider gating these behind akDebugModecheck or removing them.
🤖 Fix all issues with AI agents
In `@lib/services/supabase_service.dart`:
- Around line 349-354: The outer catch in the createTeam and
createTeamWithGoogle flows swallows the inner, more specific exception message
(e.g., the Exception thrown in the team-code uniqueness check) and always
returns a generic 'Failed to create team' error; update the outer catch in the
methods (look for createTeam and createTeamWithGoogle in supabase_service.dart)
to preserve and propagate the original error message (e.g., include e.toString()
or inspect the exception type/message and use it in the thrown/returned
Exception) so connection/RPC errors like 'Unable to verify team code uniqueness.
Please check your connection.' are surfaced to the caller instead of being
overwritten by the generic message.
- Around line 709-739: The code block starting at the unique-team-id generation
is orphaned—add a proper async method declaration above it: declare
Future<Map<String, dynamic>> createTeamWithGoogle({ required String teamName,
required String ownerId, /* add any other parameters referenced in the body */
}) async { and then ensure the existing body (including the while loop, RPC
calls like _client.rpc, try/catch blocks) is inside the method, with matching
braces and a final return statement returning a Map<String, dynamic> (or the
expected result object) and appropriate error handling; reference
createTeamWithGoogle, generateTeamId, _client.rpc, and the isUnique/attempts
variables to locate where to insert the signature and close the method.
🧹 Nitpick comments (4)
lib/services/supabase_service.dart (2)
924-952: Consistent application of the fix — looks good.The OTP
signup_createflow applies the same pattern: collision → increment + continue, RPC error → throw, post-loop guard. This is consistent with thecreateTeamfix.One minor note: the error messages differ slightly across the three instances (
'Unable to verify team code uniqueness. Please check your connection.'increateTeam,'Unable to verify team code uniqueness. Please check your connection.'increateTeamWithGoogle, and the shorter'Unable to verify team code uniqueness.'here at line 946). Consider using a consistent message for all three for uniform UX.
327-360: Consider extracting the unique team code generation into a shared helper method.The same loop pattern (generate code → check via RPC → retry on collision, fail fast on error, guard post-loop) is duplicated across
createTeam,createTeamWithGoogle, and the OTPsignup_createflow. Extracting this into a singleFuture<String> _generateUniqueTeamCode()method would reduce duplication and ensure future fixes apply uniformly.Also applies to: 709-739, 924-952
lib/services/ai_service.dart (2)
392-434: Good defensive improvement wrapping JSON decoding in try-catch.The logic correctly prioritizes function calls over text parts and falls back gracefully. One minor robustness concern: line 397 casts
responseData['candidates']directly toList<dynamic>. If the key is absent or null (e.g., a content-filtered response), the cast throws, and the catch block returns "invalid response from AI server" — which is slightly misleading vs. the more apt "No response generated" at line 426. A null-check before the cast would let you distinguish the two cases.Suggested improvement
- final candidates = responseData['candidates'] as List<dynamic>; - if (candidates.isNotEmpty) { + final candidates = responseData['candidates'] as List<dynamic>?; + if (candidates != null && candidates.isNotEmpty) {
538-552:handleToolResponselacks the same try-catch hardening applied togenerateChatResponse.Lines 539–543 perform
jsonDecodeand castcandidateswithout try-catch protection, while the parallel code path ingenerateChatResponse(lines 392–434) was just hardened. A malformed follow-up response here would crash instead of returning a safe fallback. Consider applying the same pattern for consistency.
| // Generate a unique team ID | ||
| String teamId = ''; | ||
| bool isUnique = false; | ||
| int attempts = 0; | ||
|
|
||
| do { | ||
| while (!isUnique && attempts < 10) { | ||
| teamId = generateTeamId(); | ||
| attempts++; | ||
|
|
||
| try { | ||
| final response = await _client.rpc( | ||
| 'check_team_code_exists', | ||
| params: {'code': teamId}, | ||
| ); | ||
|
|
||
| isUnique = response == false; | ||
| if (response == true) { | ||
| attempts++; | ||
| continue; | ||
| } | ||
|
|
||
| isUnique = true; | ||
| } catch (e) { | ||
| debugPrint('Error checking team code: $e'); | ||
| // If RPC consistently fails, throw error instead of risking duplicates | ||
| if (attempts >= 3) { | ||
| throw Exception( | ||
| 'Unable to verify team code uniqueness. Please try again.'); | ||
| } | ||
| throw Exception( | ||
| 'Unable to verify team code uniqueness. Please check your connection.'); | ||
| } | ||
| } while (!isUnique && attempts < 10); | ||
| } | ||
|
|
||
| if (!isUnique) { | ||
| throw Exception( | ||
| 'Failed to generate a unique team code after multiple attempts. Please try again.'); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the createTeamWithGoogle method signature exists in the file
echo "=== Searching for createTeamWithGoogle declaration ==="
rg -n 'createTeamWithGoogle' lib/services/supabase_service.dart
echo ""
echo "=== Lines 700-720 of supabase_service.dart ==="
sed -n '700,720p' lib/services/supabase_service.dartRepository: AOSSIE-Org/Ell-ena
Length of output: 688
🏁 Script executed:
#!/bin/bash
echo "=== Lines 685-750 to see full context ==="
sed -n '685,750p' lib/services/supabase_service.dart
echo ""
echo "=== Check file structure around method definitions ==="
rg -n 'Future.*createTeam|Future.*joinTeam|^\s*Future' lib/services/supabase_service.dart | head -20Repository: AOSSIE-Org/Ell-ena
Length of output: 3113
Critical: createTeamWithGoogle method signature is missing.
The code body starting at line 709 (unique team ID generation) lacks a method declaration. Line 708 contains only a comment, followed immediately by variable declarations with no return type, method name, parameters, or opening brace. This code is orphaned and will not compile.
The file jumps directly from the closing of joinTeamWithGoogle (line 706) to the uncommented code body at line 709. A search of the file confirms no createTeamWithGoogle method declaration exists anywhere.
You need to add the method signature above line 709, likely:
Future<Map<String, dynamic>> createTeamWithGoogle({
required String teamName,
// ... other parameters
}) async {
try {🤖 Prompt for AI Agents
In `@lib/services/supabase_service.dart` around lines 709 - 739, The code block
starting at the unique-team-id generation is orphaned—add a proper async method
declaration above it: declare Future<Map<String, dynamic>>
createTeamWithGoogle({ required String teamName, required String ownerId, /* add
any other parameters referenced in the body */ }) async { and then ensure the
existing body (including the while loop, RPC calls like _client.rpc, try/catch
blocks) is inside the method, with matching braces and a final return statement
returning a Map<String, dynamic> (or the expected result object) and appropriate
error handling; reference createTeamWithGoogle, generateTeamId, _client.rpc, and
the isUnique/attempts variables to locate where to insert the signature and
close the method.
150e724 to
4675760
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/services/supabase_service.dart (1)
327-355: 🛠️ Refactor suggestion | 🟠 MajorExtract the team-code generation + uniqueness check into a shared helper.
The bounded-retry + RPC-check logic is now duplicated verbatim in
createTeam,createTeamWithGoogle, and theverifyOTPsignup path. A single helper would eliminate the three copies and make future changes (e.g., adding backoff) a one-line fix.Example helper
/// Generates a unique team code, retrying up to [maxAttempts] times. /// Throws on RPC/network errors or if no unique code is found. Future<String> _generateUniqueTeamCode({int maxAttempts = 10}) async { for (int attempt = 0; attempt < maxAttempts; attempt++) { final code = generateTeamId(); try { final response = await _client.rpc( 'check_team_code_exists', params: {'code': code}, ); if (response == true) continue; // collision, retry if (response == false) return code; // unique // unexpected response debugPrint('Unexpected RPC response: $response'); } catch (e) { debugPrint('Error checking team code: $e'); throw Exception( 'Unable to verify team code uniqueness. Please check your connection.', ); } } throw Exception( 'Failed to generate a unique team code after multiple attempts.', ); }
🧹 Nitpick comments (1)
lib/services/supabase_service.dart (1)
327-360: The loop logic correctly addresses the original infinite-loop bug.The bounded
whileloop with fail-fast on RPC errors and the post-loop guard are sound improvements. Good work.One subtlety: if the RPC returns
null(e.g., unexpected schema change or edge case), theresponse == truecheck passes through toisUnique = true, treating an ambiguous result as "unique." Consider adding an explicit check forresponse == falsebefore accepting uniqueness:Suggested defensive check
if (response == true) { attempts++; continue; } - - // If response is false, the code is unique - isUnique = true; + + if (response == false) { + isUnique = true; + } else { + // Unexpected response from RPC + debugPrint('Unexpected RPC response: $response'); + attempts++; + }
1 participant
Where:
SupabaseService.createTeamIssue: The do-while loop tries 10 times to generate a unique ID. If the RPC check_team_code_exists throws an exception (e.g. network error), it counts as a failed attempt. 10 network errors = "Failed to create team".
Fix: Distinguish between "Code exists" (retry) and "Network Error" (throw immediately or retry with backoff).
@SharkyBytes Please review Closes #155
Summary by CodeRabbit
Bug Fixes