feat: add peer connections system with requests, accept/decline, and dashboard UI#29
Open
ayesha1145 wants to merge 3 commits intoalphaonelabs:mainfrom
Open
feat: add peer connections system with requests, accept/decline, and dashboard UI#29ayesha1145 wants to merge 3 commits intoalphaonelabs:mainfrom
ayesha1145 wants to merge 3 commits intoalphaonelabs:mainfrom
Conversation
…a-one-labs-logo Add Alpha One Labs logo to header and favicon across all pages
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a peer-connections feature: dashboard UI and client JS for tabbed peer lists and actions, a new Changes
Sequence DiagramsequenceDiagram
participant Browser as Browser
participant Server as Server (worker.py)
participant DB as Database (SQLite)
Browser->>Server: GET /api/peers/requests + Authorization: Bearer <JWT>
Server->>Server: Verify JWT -> user_id
Server->>DB: SELECT peer_connections WHERE addressee_id=? AND status='pending'
DB-->>Server: pending request rows
Server->>DB: SELECT user details for requester_ids
DB-->>Server: requester user rows
Server-->>Browser: 200 { ok:true, requests:[...] }
sequenceDiagram
participant Browser as Browser
participant Server as Server (worker.py)
participant DB as Database (SQLite)
Browser->>Server: POST /api/peers/request/{addressee_id} + body {message} + Authorization: Bearer <JWT>
Server->>Server: Verify JWT -> requester_id
Server->>DB: SELECT shared active enrollments AND existing peer_connections (either direction)
DB-->>Server: enrollments / existing rows
Server->>Server: Validate (not self, has shared enrollment, no conflicting row)
Server->>DB: INSERT peer_connections (status='pending', message...)
DB-->>Server: Insert OK / error
Server-->>Browser: 200 { ok:true, connection_id } or error code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@public/dashboard.html`:
- Around line 186-190: The tab group is missing ARIA roles and attributes and
the JS does not update them; add role="tablist" and aria-label on the container
and set role="tab", aria-selected and aria-controls on the three buttons (ids:
tab-connections, tab-requests, tab-find) and add corresponding tabpanel elements
with ids peers-connections, peers-requests, peers-find and role="tabpanel" +
aria-labelledby; then update the showPeerTab(tab) function to toggle the visible
panel classes and set aria-selected="true"/"false" on the buttons and update
each panel's aria-hidden (or equivalent) so screen readers reflect the active
tab.
- Around line 178-179: The peers section (the div with id="peers-section") is
currently located after the footer; move that entire element so it becomes a
sibling of the element with id="joined-section" inside the <main> container,
ensuring it appears before the footer in the DOM and preserves its classes and
content, so semantic structure and accessibility are corrected.
- Around line 178-207: Define the missing UI and data functions referenced by
the peer section: implement showPeerTab(tabName) to switch visible panels and
active tab classes for IDs tab-connections/tab-requests/tab-find and panels
peers-connections/peers-requests/peers-find; add async loader functions
loadPeers(), loadPeerRequests(), loadSuggestions() that fetch data and call
render helpers renderPeers(data), renderRequests(data), renderSuggestions(data)
to populate the containers peers-list, requests-list, suggestions-list; add
escP() (or rename uses to esc() if intended) to match the PR objective; ensure
these functions are registered before the HTML buttons run (put them in the page
<script> and call initializers like loadPeers() on DOMContentLoaded); finally,
move the peer connections block into the <main> element for semantic
correctness.
In `@src/worker.py`:
- Around line 1445-1447: Wrap the UPDATE calls that use
env.DB.prepare(...).bind(connection_id).run() inside a try/except and handle DB
errors consistently with other write operations (e.g., in
api_accept_peer_request and api_decline_peer_request): catch the exception
around the env.DB.prepare(...) call, call processLogger.error or the existing
logger with a descriptive message that includes the exception details and the
connection_id, and return/raise an appropriate error response so the caller
knows the update failed; mirror the same pattern for the UPDATE at the other
location referenced.
- Around line 1407-1429: Existing declined requests fall through to the INSERT
and trigger a UNIQUE constraint error; update the logic in the handler around
the existing lookup (the existing variable and the peer_connections queries) to
handle existing["status"] == "declined" explicitly: either return a clear client
error (e.g., err("Previous request was declined", 409)) or perform an UPDATE on
the existing row (use env.DB.prepare(...) to UPDATE peer_connections SET
requester_id=?, addressee_id=?, message=?, status='pending' WHERE id=? using
existing["id"] and the new message) instead of INSERT so resends succeed; adjust
conn_id usage accordingly and keep capture_exception("api_send_peer_request") in
the exception path as before.
- Around line 1362-1368: The loop building peers uses dict-style access
(r["id"], r["peer_id"], etc.) which is inconsistent with other handlers; change
to attribute-style access (e.g., r.id, r.peer_id, r.peer_name_enc, r.created_at)
when constructing the peer dict in the same loop, but keep the current
async-await pattern for decrypt_aes (do not convert to a list comprehension).
Update the keys in the constructed dict to use these attribute accesses
(connection_id: r.id, peer_id: r.peer_id, peer_name: await
decrypt_aes(r.peer_name_enc or "", enc), connected_at: r.created_at) to match
the codebase style.
- Around line 1427-1428: The exception handler inside api_send_peer_request
currently calls capture_exception(exc, where="api_send_peer_request") without
request context; update the call to pass the current req and env objects (e.g.,
capture_exception(exc, where="api_send_peer_request", req=req, env=env)) so logs
include the same request/environment context other handlers provide; locate the
except block in api_send_peer_request and add the req and env kwargs when
calling capture_exception.
🪄 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: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a347f21c-22de-4e94-88b2-c4fa210ef7f5
📒 Files selected for processing (3)
public/dashboard.htmlschema.sqlsrc/worker.py
ayesha1145
force-pushed
the
feat/peer-connections
branch
from
580d267 to
107e2f7
Compare
Contributor
Author
|
Addressed CodeRabbit feedback:
|
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
src/worker.py (1)
1362-1368: 🧹 Nitpick | 🔵 TrivialInconsistent result access pattern — prefer attribute-style for consistency.
The code uses dictionary-style access (
r["id"],r["peer_id"]) while other handlers in this file use attribute-style (r.id,row.title). Aligning with the codebase convention improves readability and maintainability.♻️ Suggested fix
for r in rows.results or []: peers.append({ - "connection_id": r["id"], - "peer_id": r["peer_id"], - "peer_name": await decrypt_aes(r["peer_name_enc"] or "", enc), - "connected_at": r["created_at"], + "connection_id": r.id, + "peer_id": r.peer_id, + "peer_name": await decrypt_aes(r.peer_name_enc or "", enc), + "connected_at": r.created_at, })Note: The static analysis hint about list comprehensions doesn't apply here since
decrypt_aesis async, requiring the loop structure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/worker.py` around lines 1362 - 1368, Change the dict-style field access on rows.results to attribute-style to match the rest of the file: inside the loop over rows.results use r.id, r.peer_id, r.peer_name_enc (or fallback empty string) and r.created_at when building the dict appended to peers; keep the async decrypt_aes(r.peer_name_enc or "", enc) call and the loop structure (do not convert to a list comprehension) so behavior remains the same.public/dashboard.html (3)
70-72:⚠️ Potential issue | 🔴 CriticalMissing JavaScript implementation for peer connections UI.
The tab buttons call
showPeerTab()(lines 70-72), but this function is not defined anywhere in the<script>block. Clicking any tab will throwReferenceError: showPeerTab is not defined, making the peer connections feature non-functional.The following functions need to be implemented:
showPeerTab(tabName)— switch visible panels and updatearia-selectedstatesloadPeers()— fetch from/api/peersand populate#peers-listloadPeerRequests()— fetch from/api/peers/requestsand populate#requests-list- Functions to handle accept/decline/connect actions
Would you like me to help generate the JavaScript implementation for these functions?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/dashboard.html` around lines 70 - 72, Implement the missing peer UI JavaScript by adding definitions for showPeerTab(tabName), loadPeers(), loadPeerRequests(), and action handlers (acceptPeerRequest, declinePeerRequest, connectToPeer): showPeerTab should toggle visibility of the tab panels (e.g., panels with ids like "connections", "requests", "find"), update the aria-selected attribute and CSS classes on the tab buttons (ids tab-connections, tab-requests, tab-find) to reflect the active tab; loadPeers() should fetch GET /api/peers, render the results into the `#peers-list` container and attach connect button handlers that call connectToPeer(id); loadPeerRequests() should fetch GET /api/peers/requests, render into `#requests-list` and attach acceptPeerRequest(id) and declinePeerRequest(id) handlers which POST/DELETE to the appropriate endpoints and refresh lists on success; ensure all fetch calls handle errors and update the UI state (disable buttons while pending, show simple error text), and call loadPeers() and loadPeerRequests() on initial script run and after actions to keep the UI in sync.
76-89: 🧹 Nitpick | 🔵 TrivialAdd
role="tabpanel"andaria-labelledbyto tab panels.To complete the accessible tabs pattern, each panel should be marked as a
tabpaneland linked to its controlling tab button. This helps screen readers understand the relationship between tabs and their content.♿ Suggested enhancement
<!-- My Connections --> - <div id="peers-connections" class="peer-panel"> + <div id="peers-connections" class="peer-panel" role="tabpanel" aria-labelledby="tab-connections"> <div id="peers-list" class="space-y-3"></div> </div> <!-- Incoming Requests --> - <div id="peers-requests" class="peer-panel hidden"> + <div id="peers-requests" class="peer-panel hidden" role="tabpanel" aria-labelledby="tab-requests"> <div id="requests-list" class="space-y-3"></div> </div> <!-- Find Peers --> - <div id="peers-find" class="peer-panel hidden"> + <div id="peers-find" class="peer-panel hidden" role="tabpanel" aria-labelledby="tab-find"> <p class="text-sm text-slate-500 mb-3">Connect with other learners from your activities.</p> <div id="suggestions-list" class="space-y-3"></div> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/dashboard.html` around lines 76 - 89, Add role="tabpanel" and an aria-labelledby that points to the controlling tab button for each panel div (peers-connections, peers-requests, peers-find); for example set aria-labelledby="peers-connections-tab", "peers-requests-tab", and "peers-find-tab" respectively, and make sure the corresponding tab buttons use those IDs so the relationship is valid; also keep the existing hidden toggling (e.g., class "hidden") in sync with the tab logic (update aria-hidden as you toggle if your tab script manages visibility).
69-73: 🧹 Nitpick | 🔵 TrivialEnhance tab container with
role="tablist"for full accessibility compliance.The individual tab buttons correctly include
role="tab"andaria-selected, which is great progress! However, the container<div>on line 69 should haverole="tablist"and anaria-labelto complete the accessible tabs pattern. Additionally, each button would benefit fromaria-controlspointing to its corresponding panel.♿ Suggested enhancement
- <div class="flex gap-2 mb-4 border-b border-slate-100"> - <button type="button" role="tab" aria-selected="true" onclick="showPeerTab('connections')" id="tab-connections" class="peer-tab px-4 py-2 text-sm font-semibold text-indigo-600 border-b-2 border-indigo-600">My Peers</button> - <button type="button" role="tab" aria-selected="false" onclick="showPeerTab('requests')" id="tab-requests" class="peer-tab px-4 py-2 text-sm font-semibold text-slate-400 border-b-2 border-transparent">Requests</button> - <button type="button" role="tab" aria-selected="false" onclick="showPeerTab('find')" id="tab-find" class="peer-tab px-4 py-2 text-sm font-semibold text-slate-400 border-b-2 border-transparent">Find Peers</button> + <div class="flex gap-2 mb-4 border-b border-slate-100" role="tablist" aria-label="Peer connection views"> + <button type="button" role="tab" aria-selected="true" aria-controls="peers-connections" onclick="showPeerTab('connections')" id="tab-connections" class="peer-tab px-4 py-2 text-sm font-semibold text-indigo-600 border-b-2 border-indigo-600">My Peers</button> + <button type="button" role="tab" aria-selected="false" aria-controls="peers-requests" onclick="showPeerTab('requests')" id="tab-requests" class="peer-tab px-4 py-2 text-sm font-semibold text-slate-400 border-b-2 border-transparent">Requests</button> + <button type="button" role="tab" aria-selected="false" aria-controls="peers-find" onclick="showPeerTab('find')" id="tab-find" class="peer-tab px-4 py-2 text-sm font-semibold text-slate-400 border-b-2 border-transparent">Find Peers</button>As per coding guidelines, "Review HTML templates for accessibility (ARIA attributes, semantic elements)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/dashboard.html` around lines 69 - 73, Add accessibility attributes to the tabs container and buttons: give the container div (the element wrapping the three buttons) role="tablist" and an appropriate aria-label (e.g., "Peer tabs"); add aria-controls to each tab button (tab-connections, tab-requests, tab-find) pointing to their corresponding panel IDs (use a consistent naming convention such as panel-connections, panel-requests, panel-find); ensure the corresponding panels exist and have role="tabpanel", the matching id (panel-...), and aria-labelledby that references the controlling button id (tab-connections, tab-requests, tab-find) so the showPeerTab function continues to toggle visible panels while meeting ARIA tabs pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@schema.sql`:
- Around line 110-112: Add a composite index to speed the common query in
api_list_peer_requests that filters WHERE addressee_id=? AND status='pending':
create an index on peer_connections covering (addressee_id, status) (e.g.,
idx_pc_addressee_status) so the planner can use a single index for that
predicate instead of two separate single-column indexes; add the CREATE INDEX IF
NOT EXISTS statement for idx_pc_addressee_status to schema.sql and keep or
remove the separate single-column indexes as you prefer.
- Around line 98-109: Add a CHECK constraint on the peer_connections.status
column to restrict values to 'pending', 'accepted', or 'declined': update the
CREATE TABLE peer_connections definition to include CHECK(status IN
('pending','accepted','declined')) for the status column (or add the same
constraint via ALTER TABLE for migrations) so the database enforces the allowed
states; reference the peer_connections table and the status column when applying
the change and ensure any migration scripts handle existing invalid rows before
adding the constraint.
In `@src/worker.py`:
- Around line 1386-1393: The loop building requests uses dict-style access on
the row object; switch to attribute access for consistency—change r["id"] ->
r.id, r["requester_id"] -> r.requester_id, r["requester_name_enc"] ->
r.requester_name_enc, r["message"] -> r.message, and r["created_at"] ->
r.created_at in the requests.append block (keeping the await
decrypt_aes(r.requester_name_enc or "", enc) call and the same keys
"connection_id", "requester_id", "requester_name", "message", "requested_at").
- Around line 1445-1446: The code uses dict-style access pc["status"] instead of
attribute-style access used elsewhere; update all occurrences (e.g., inside the
request handling where pc["status"] is checked and in the functions
api_send_peer_request and api_decline_peer_request) to use attribute access
pc.status, keeping the same comparison and behavior (e.g., if pc.status !=
"pending": return err(...)); ensure you replace every pc["status"] instance in
these functions with pc.status to maintain consistency.
- Line 1490: Wrap the await env.DB.prepare("DELETE FROM peer_connections WHERE
id=?").bind(connection_id).run() call in a try/except block to mirror the other
write operations: catch Exception as e, log a clear error message including the
connection_id and the exception (using the same logger used elsewhere in this
file), and then either re-raise or handle the error consistently with existing
write-operation error handling in this module.
---
Duplicate comments:
In `@public/dashboard.html`:
- Around line 70-72: Implement the missing peer UI JavaScript by adding
definitions for showPeerTab(tabName), loadPeers(), loadPeerRequests(), and
action handlers (acceptPeerRequest, declinePeerRequest, connectToPeer):
showPeerTab should toggle visibility of the tab panels (e.g., panels with ids
like "connections", "requests", "find"), update the aria-selected attribute and
CSS classes on the tab buttons (ids tab-connections, tab-requests, tab-find) to
reflect the active tab; loadPeers() should fetch GET /api/peers, render the
results into the `#peers-list` container and attach connect button handlers that
call connectToPeer(id); loadPeerRequests() should fetch GET /api/peers/requests,
render into `#requests-list` and attach acceptPeerRequest(id) and
declinePeerRequest(id) handlers which POST/DELETE to the appropriate endpoints
and refresh lists on success; ensure all fetch calls handle errors and update
the UI state (disable buttons while pending, show simple error text), and call
loadPeers() and loadPeerRequests() on initial script run and after actions to
keep the UI in sync.
- Around line 76-89: Add role="tabpanel" and an aria-labelledby that points to
the controlling tab button for each panel div (peers-connections,
peers-requests, peers-find); for example set
aria-labelledby="peers-connections-tab", "peers-requests-tab", and
"peers-find-tab" respectively, and make sure the corresponding tab buttons use
those IDs so the relationship is valid; also keep the existing hidden toggling
(e.g., class "hidden") in sync with the tab logic (update aria-hidden as you
toggle if your tab script manages visibility).
- Around line 69-73: Add accessibility attributes to the tabs container and
buttons: give the container div (the element wrapping the three buttons)
role="tablist" and an appropriate aria-label (e.g., "Peer tabs"); add
aria-controls to each tab button (tab-connections, tab-requests, tab-find)
pointing to their corresponding panel IDs (use a consistent naming convention
such as panel-connections, panel-requests, panel-find); ensure the corresponding
panels exist and have role="tabpanel", the matching id (panel-...), and
aria-labelledby that references the controlling button id (tab-connections,
tab-requests, tab-find) so the showPeerTab function continues to toggle visible
panels while meeting ARIA tabs pattern.
In `@src/worker.py`:
- Around line 1362-1368: Change the dict-style field access on rows.results to
attribute-style to match the rest of the file: inside the loop over rows.results
use r.id, r.peer_id, r.peer_name_enc (or fallback empty string) and r.created_at
when building the dict appended to peers; keep the async
decrypt_aes(r.peer_name_enc or "", enc) call and the loop structure (do not
convert to a list comprehension) so behavior remains the same.
🪄 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: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 65a1231d-9ddb-4808-8584-dd33b566ea65
📒 Files selected for processing (3)
public/dashboard.htmlschema.sqlsrc/worker.py
ayesha1145
force-pushed
the
feat/peer-connections
branch
from
107e2f7 to
07012fb
Compare
Contributor
Author
|
Fixed critical: JS functions (showPeerTab, loadPeers, renderPeers, renderRequests, acceptRequest, declineRequest, removePeer, loadSuggestions, escP) were missing from the script block — now properly added inside <script> tag. All peer connections UI functions are now fully implemented. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
public/dashboard.html (2)
8-10:⚠️ Potential issue | 🔴 CriticalSplit the Tailwind CDN include from the peer script block.
Because Line 8 has a
srcattribute, browsers ignore the inline JavaScript inside this element. The peer functions in this block never get defined, so the runtime failure is still present. Keep the Tailwind include empty and move this logic into its own<script>tag.In HTML, if a <script> element has a src attribute, does the inline JavaScript inside that same element execute or is it ignored by browsers?Also applies to: 122-122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/dashboard.html` around lines 8 - 10, The Tailwind <script> element currently contains a src attribute and inline JS (the peer functions), which browsers ignore; split them by leaving the Tailwind include as a single empty <script src="https://cdn.tailwindcss.com"></script> and move all inline peer functions into a separate <script> block immediately after it (i.e., relocate the code currently inside the script with src into its own <script> so functions are defined at runtime); apply the same change to the other occurrence noted (line ~122).
65-77:⚠️ Potential issue | 🟡 MinorFinish wiring the tablist/panel relationships.
The buttons now expose
role="tab"andaria-selected, but the group still lacksrole="tablist", the tabs have noaria-controls, and the panels have norole="tabpanel"/aria-labelledby. Please also togglearia-hiddenalongside the visualhiddenstate inshowPeerTab().As per coding guidelines,
**/*.html: Review HTML templates for accessibility (ARIA attributes, semantic elements).`Also applies to: 181-200
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/dashboard.html` around lines 65 - 77, Add the missing ARIA relationships and toggle aria-hidden in the tab logic: give the tabs container role="tablist", add aria-controls on each tab button referencing its panel id (e.g., tab buttons with id "tab-<name>" should have aria-controls="peers-<name>"), add role="tabpanel" and aria-labelledby on each panel (panels with id "peers-<name>" should have aria-labelledby="tab-<name>"), and update showPeerTab() to set aria-hidden="true" on panels being hidden and aria-hidden="false" on the visible panel in addition to adding/removing the CSS "hidden" class; apply the same attribute fixes to the second tablist/panel block referenced around lines 181-200.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@public/dashboard.html`:
- Around line 80-89: loadSuggestions currently only renders joined_activities
title/host_name and lacks any user identifier, connection state, or action to
call the peer-request endpoint; update the function to (1) expect and use a
candidate user_id from the API response (e.g., include user_id on each joined
activity), (2) render each candidate with a request/connect button (attach
data-user-id or id attribute) and a visual connection state element, and (3)
wire that button to an async handler that posts to POST /api/peers/request/:id
(use fetch with Authorization header) and updates the UI state on success/error;
reference loadSuggestions, joined_activities, escP and the suggestions-list
element when locating where to add the user_id rendering, the button, and the
click handler.
In `@schema.sql`:
- Around line 98-109: Change the one-direction UNIQUE on peer_connections to
enforce a canonical unordered pair by adding computed/stored columns that
normalize the pair and a UNIQUE constraint on those columns: in the CREATE TABLE
for peer_connections, add two generated columns (e.g., user_a and user_b) that
set user_a = CASE WHEN requester_id < addressee_id THEN requester_id ELSE
addressee_id END and user_b = CASE WHEN requester_id < addressee_id THEN
addressee_id ELSE requester_id END, then replace UNIQUE(requester_id,
addressee_id) with UNIQUE(user_a, user_b); also make the identical change in
src/worker.py’s _DDL definition so the same DDL is used by the worker. Ensure
foreign keys and other column definitions remain unchanged.
In `@src/worker.py`:
- Around line 1404-1410: The code currently only checks the addressee exists;
before checking/creating peer_connections you must enforce the shared-activity
rule by querying the DB (using env.DB.prepare and bind with addressee_id and
user["id"]) to confirm both users are members of at least one common learning
activity; insert this check immediately after the addressee lookup and before
the existing peer_connections query, and if the query finds no shared activity
return an error (e.g., err("Users do not share a learning activity", 403)).
- Around line 1418-1422: The handler currently rejects empty POST bodies because
parse_json_object returning a falsy/empty body leads to returning bad; instead,
treat an absent/empty body as an empty dict so message remains optional. Modify
the code around parse_json_object so that if parse_json_object yields a
None/empty body (or bad indicates an empty body) you set body = {} (and do not
return bad), then continue to extract raw_msg and compute message as you already
do; adjust logic that uses bad so only truly malformed JSON returns bad. Update
references to parse_json_object, body, bad, raw_msg and message accordingly.
---
Duplicate comments:
In `@public/dashboard.html`:
- Around line 8-10: The Tailwind <script> element currently contains a src
attribute and inline JS (the peer functions), which browsers ignore; split them
by leaving the Tailwind include as a single empty <script
src="https://cdn.tailwindcss.com"></script> and move all inline peer functions
into a separate <script> block immediately after it (i.e., relocate the code
currently inside the script with src into its own <script> so functions are
defined at runtime); apply the same change to the other occurrence noted (line
~122).
- Around line 65-77: Add the missing ARIA relationships and toggle aria-hidden
in the tab logic: give the tabs container role="tablist", add aria-controls on
each tab button referencing its panel id (e.g., tab buttons with id "tab-<name>"
should have aria-controls="peers-<name>"), add role="tabpanel" and
aria-labelledby on each panel (panels with id "peers-<name>" should have
aria-labelledby="tab-<name>"), and update showPeerTab() to set
aria-hidden="true" on panels being hidden and aria-hidden="false" on the visible
panel in addition to adding/removing the CSS "hidden" class; apply the same
attribute fixes to the second tablist/panel block referenced around lines
181-200.
🪄 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: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0a779800-2ac1-4908-9fb6-26f0c7dce1a2
📒 Files selected for processing (3)
public/dashboard.htmlschema.sqlsrc/worker.py
ayesha1145
force-pushed
the
feat/peer-connections
branch
from
07012fb to
584f2ff
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
schema.sql (2)
110-112: 🧹 Nitpick | 🔵 TrivialConsider a composite index for the common pending-requests query.
The
api_list_peer_requestshandler filters byWHERE addressee_id=? AND status='pending'. A composite index would serve this pattern more efficiently:CREATE INDEX IF NOT EXISTS idx_pc_addressee_status ON peer_connections(addressee_id, status);This is an optional optimization — the current separate indexes will work fine for typical user volumes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema.sql` around lines 110 - 112, Add a composite index to speed up the api_list_peer_requests pattern by creating an index on peer_connections(addressee_id, status) (suggested name idx_pc_addressee_status) so queries filtering WHERE addressee_id=? AND status='pending' can use a single index instead of two separate indexes; add the CREATE INDEX IF NOT EXISTS idx_pc_addressee_status ON peer_connections(addressee_id, status) statement alongside the existing indexes.
98-109:⚠️ Potential issue | 🟠 MajorBidirectional uniqueness enforcement is still missing.
The
UNIQUE (requester_id, addressee_id)constraint only prevents duplicate requests in one direction. Two concurrent requests could still create bothA→BandB→Arows, leading to duplicate relationships that the API doesn't expect since connections are treated as undirected once accepted.Consider enforcing canonical ordering at the database layer using generated columns:
user_a TEXT GENERATED ALWAYS AS (CASE WHEN requester_id < addressee_id THEN requester_id ELSE addressee_id END) STORED, user_b TEXT GENERATED ALWAYS AS (CASE WHEN requester_id < addressee_id THEN addressee_id ELSE requester_id END) STORED, UNIQUE(user_a, user_b)Alternatively, the application layer in
api_send_peer_requestalready checks both directions before inserting (lines 1447-1450 in worker.py), which mitigates this for non-concurrent requests. For full protection, the database constraint would be ideal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema.sql` around lines 98 - 109, The peer_connections table allows A→B and B→A rows because UNIQUE(requester_id, addressee_id) is directional; add canonical ordering at the DB layer by introducing generated stored columns (e.g., user_a and user_b computed from requester_id and addressee_id using CASE to pick the lesser/greater) and then add UNIQUE(user_a, user_b) to enforce bidirectional uniqueness for the peer_connections table; reference the peer_connections table and the requester_id/addressee_id columns and add user_a/user_b GENERATED ALWAYS AS (...) STORED plus UNIQUE(user_a, user_b) (or alternatively implement an equivalent DB trigger) to prevent concurrent opposite-direction inserts.src/worker.py (2)
1369-1374: 🧹 Nitpick | 🔵 TrivialDictionary-style vs attribute-style access inconsistency.
The peer connection handlers use dictionary-style access (
r["user_id"],r["id"]) while other handlers in this file use attribute-style access (row.id,r.title). Both work with D1 results, but consistency aids maintainability.This applies throughout the new handlers (lines 1369-1374, 1393-1399, 1417-1423, 1452, 1488, 1510). Consider aligning with the existing codebase pattern using attribute access.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/worker.py` around lines 1369 - 1374, The loop building suggestions uses dict-style access (r["user_id"], r["name_enc"], r["activity_title"]) which is inconsistent with the rest of the file; change these to attribute-style access (e.g., r.user_id, r.name_enc, r.activity_title) and keep the same None/empty fallback for decrypt_aes (e.g., decrypt_aes(r.name_enc or "", enc)); apply the same conversion in the other new handlers that build lists (the other rows.results loops that call suggestions.append or similar) so all D1 result access uses attribute access consistently.
1522-1534: 🧹 Nitpick | 🔵 TrivialAuthorization looks correct; consider optional error handling for consistency.
The
api_remove_peerhandler properly restricts deletion to participants of accepted connections only. The query on lines 1527-1530 correctly checks that the authenticated user is either the requester or addressee.For consistency with the accept/decline handlers, you could wrap the DELETE in try/except:
🛡️ Optional enhancement
+ try: await env.DB.prepare("DELETE FROM peer_connections WHERE id=?").bind(connection_id).run() + except Exception as exc: + capture_exception(exc, req, env, "api_remove_peer.delete") + return err("Failed to remove connection", 500) return ok(msg="Connection removed")This is a nice-to-have for consistency rather than critical.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/worker.py` around lines 1522 - 1534, The DELETE in api_remove_peer should be wrapped in a try/except to mirror accept/decline handlers and surface DB errors consistently; catch exceptions around the env.DB.prepare("DELETE FROM peer_connections WHERE id=?").bind(connection_id).run() call, log or return a 500-style err with the error message, and ensure the handler still returns ok(msg="Connection removed") on success while returning an err("Failed to remove connection", 500) (including error details) on exception.public/dashboard.html (1)
206-226:⚠️ Potential issue | 🟡 MinorAdd
role="tabpanel"andaria-labelledbyto tab panels for complete accessibility.The tab buttons correctly have
role="tab"andaria-selectedattributes, but the corresponding panels are missing their ARIA roles. This limits accessibility for screen reader users.♿ Proposed accessibility enhancement
- <div id="peers-connections" class="peer-panel"> + <div id="peers-connections" class="peer-panel" role="tabpanel" aria-labelledby="tab-connections"> - <div id="peers-requests" class="peer-panel hidden"> + <div id="peers-requests" class="peer-panel hidden" role="tabpanel" aria-labelledby="tab-requests"> - <div id="peers-find" class="peer-panel hidden"> + <div id="peers-find" class="peer-panel hidden" role="tabpanel" aria-labelledby="tab-find">As per coding guidelines, HTML templates should include ARIA attributes for accessibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/dashboard.html` around lines 206 - 226, Add proper ARIA semantics to each tab panel by giving the elements with ids peers-connections, peers-requests, and peers-find the attribute role="tabpanel" and an aria-labelledby that references the corresponding tab button ids (tab-connections, tab-requests, tab-find respectively); ensure the panels' visibility toggling logic (showPeerTab) continues to manage the "hidden" class and aria-hidden as needed so screen readers see the active tab panel via its role and labelled-by relationship.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@public/dashboard.html`:
- Around line 8-146: The inline peer-connection functions (loadPeers,
renderPeers, renderRequests, escP, showPeerTab, loadSuggestions, sendRequest,
acceptRequest, declineRequest, removePeer) are currently inside a <script> tag
that has a src attribute so browsers ignore them; move all that inline JS into a
separate <script> block (or external file) so the functions are actually
defined, and ensure the token variable is defined before these functions run —
either place the new script after the script that sets token or wrap
initialization calls (e.g., loadPeers and any immediate usage of token) inside
DOMContentLoaded or a function that reads token at call-time to avoid
ReferenceError.
In `@src/worker.py`:
- Around line 453-469: The in-memory initialization constant _DDL contains the
CREATE TABLE for peer_connections and must match the updated schema.sql change
that enforces bidirectional uniqueness; update the peer_connections CREATE TABLE
string inside _DDL to use the same uniqueness constraint/logic you added in
schema.sql (e.g., replace UNIQUE (requester_id, addressee_id) with the
bidirectional uniqueness expression you used such as UNIQUE (least(requester_id,
addressee_id), greatest(requester_id, addressee_id)) or the equivalent
index-based approach), and ensure the related index names (idx_pc_requester,
idx_pc_addressee, idx_pc_status) remain consistent with the table definition.
---
Duplicate comments:
In `@public/dashboard.html`:
- Around line 206-226: Add proper ARIA semantics to each tab panel by giving the
elements with ids peers-connections, peers-requests, and peers-find the
attribute role="tabpanel" and an aria-labelledby that references the
corresponding tab button ids (tab-connections, tab-requests, tab-find
respectively); ensure the panels' visibility toggling logic (showPeerTab)
continues to manage the "hidden" class and aria-hidden as needed so screen
readers see the active tab panel via its role and labelled-by relationship.
In `@schema.sql`:
- Around line 110-112: Add a composite index to speed up the
api_list_peer_requests pattern by creating an index on
peer_connections(addressee_id, status) (suggested name idx_pc_addressee_status)
so queries filtering WHERE addressee_id=? AND status='pending' can use a single
index instead of two separate indexes; add the CREATE INDEX IF NOT EXISTS
idx_pc_addressee_status ON peer_connections(addressee_id, status) statement
alongside the existing indexes.
- Around line 98-109: The peer_connections table allows A→B and B→A rows because
UNIQUE(requester_id, addressee_id) is directional; add canonical ordering at the
DB layer by introducing generated stored columns (e.g., user_a and user_b
computed from requester_id and addressee_id using CASE to pick the
lesser/greater) and then add UNIQUE(user_a, user_b) to enforce bidirectional
uniqueness for the peer_connections table; reference the peer_connections table
and the requester_id/addressee_id columns and add user_a/user_b GENERATED ALWAYS
AS (...) STORED plus UNIQUE(user_a, user_b) (or alternatively implement an
equivalent DB trigger) to prevent concurrent opposite-direction inserts.
In `@src/worker.py`:
- Around line 1369-1374: The loop building suggestions uses dict-style access
(r["user_id"], r["name_enc"], r["activity_title"]) which is inconsistent with
the rest of the file; change these to attribute-style access (e.g., r.user_id,
r.name_enc, r.activity_title) and keep the same None/empty fallback for
decrypt_aes (e.g., decrypt_aes(r.name_enc or "", enc)); apply the same
conversion in the other new handlers that build lists (the other rows.results
loops that call suggestions.append or similar) so all D1 result access uses
attribute access consistently.
- Around line 1522-1534: The DELETE in api_remove_peer should be wrapped in a
try/except to mirror accept/decline handlers and surface DB errors consistently;
catch exceptions around the env.DB.prepare("DELETE FROM peer_connections WHERE
id=?").bind(connection_id).run() call, log or return a 500-style err with the
error message, and ensure the handler still returns ok(msg="Connection removed")
on success while returning an err("Failed to remove connection", 500) (including
error details) on exception.
🪄 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: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e841c69f-5961-49f2-8884-3e0f476eec62
📒 Files selected for processing (3)
public/dashboard.htmlschema.sqlsrc/worker.py
ayesha1145
force-pushed
the
feat/peer-connections
branch
from
584f2ff to
124ace8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
public/dashboard.html (1)
68-88:⚠️ Potential issue | 🟡 MinorFinish the tab semantics for screen readers.
The buttons now expose
role="tab", but the container still has norole="tablist"and the panels are missingrole="tabpanel"plus thearia-controls/aria-labelledbyrelationship.showPeerTab()only flips CSS classes andaria-selected, so assistive tech never gets the active panel state. Please wire the full tabs pattern here, including panel visibility state (aria-hiddenor equivalent) when switching tabs.As per coding guidelines,
**/*.html: Review HTML templates for accessibility (ARIA attributes, semantic elements), XSS risks from unescaped user content, and proper use of template inheritance. Check that forms include CSRF protection tokens.Also applies to: 266-279
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/dashboard.html` around lines 68 - 88, The tab container lacks role="tablist" and the panels lack role="tabpanel" and ARIA relationships; update the tab container div to include role="tablist", add aria-controls on each button (tab-connections -> peers-connections, tab-requests -> peers-requests, tab-find -> peers-find), and add role="tabpanel" and aria-labelledby on each panel pointing back to its button id; then update the showPeerTab(tabName) function to toggle aria-selected (true/false), set tabindex=0 for the active button and -1 for others, and toggle panel visibility via aria-hidden (false for active panel, true for others) in addition to the existing CSS class changes so assistive tech receives correct active-tab and active-panel state.schema.sql (1)
98-109:⚠️ Potential issue | 🟠 MajorEnforce uniqueness on the unordered user pair.
UNIQUE (requester_id, addressee_id)still allows concurrentA→BandB→Arows, even though the feature treats the relationship as undirected and already checks both directions insrc/worker.pyaround Lines 1447-1450. That can leave duplicate connections the list/remove flows do not expect. Please canonicalize the pair at the database layer here, and apply the same DDL change insrc/worker.pyLines 454-465.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema.sql` around lines 98 - 109, The peer_connections table allows both A→B and B→A duplicates; modify the DDL to canonicalize the unordered pair and enforce uniqueness on that canonical form by adding a generated/computed column (e.g., canonical_pair computed from requester_id and addressee_id with a deterministic ordering like min/max or lexicographic concatenation) and replace UNIQUE(requester_id, addressee_id) with UNIQUE(canonical_pair); also update the identical DDL string in src/worker.py (the table-creation SQL around the peer_connections creation) so the runtime schema matches the DB constraint.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/worker.py`:
- Around line 1355-1367: The query built in env.DB.prepare selects a.title so
DISTINCT still returns the same user multiple times when they share multiple
activities; change the SQL to deduplicate by user (e.g., GROUP BY u.id,
u.name_enc) and pick or aggregate a representative activity label (e.g.,
MIN(a.title) or GROUP_CONCAT(a.title) AS activity_title) before applying LIMIT
20, then keep the same .bind(user["id"], user["id"], user["id"],
user["id"]).all() call so the dashboard rendering (which expects user_id,
name_enc, activity_title) receives at most 20 unique users.
- Around line 1458-1465: The current code swallows any JSON parse error or
non-object body and treats it as an empty request; change the parsing so that an
empty request body is allowed but malformed JSON or a non-object JSON returns a
400. Read the raw text first (e.g., await req.text()), treat empty/whitespace
text as body = {}, otherwise attempt json.loads/await req.json() inside a
try/except and on exception return a 400 response; after parsing, if body is not
a dict return 400; then continue using body.get("message"), raw_msg and message
as before. Ensure the error path uses the same handler for POST
/api/peers/request/:id and preserves the existing message trimming logic.
---
Duplicate comments:
In `@public/dashboard.html`:
- Around line 68-88: The tab container lacks role="tablist" and the panels lack
role="tabpanel" and ARIA relationships; update the tab container div to include
role="tablist", add aria-controls on each button (tab-connections ->
peers-connections, tab-requests -> peers-requests, tab-find -> peers-find), and
add role="tabpanel" and aria-labelledby on each panel pointing back to its
button id; then update the showPeerTab(tabName) function to toggle aria-selected
(true/false), set tabindex=0 for the active button and -1 for others, and toggle
panel visibility via aria-hidden (false for active panel, true for others) in
addition to the existing CSS class changes so assistive tech receives correct
active-tab and active-panel state.
In `@schema.sql`:
- Around line 98-109: The peer_connections table allows both A→B and B→A
duplicates; modify the DDL to canonicalize the unordered pair and enforce
uniqueness on that canonical form by adding a generated/computed column (e.g.,
canonical_pair computed from requester_id and addressee_id with a deterministic
ordering like min/max or lexicographic concatenation) and replace
UNIQUE(requester_id, addressee_id) with UNIQUE(canonical_pair); also update the
identical DDL string in src/worker.py (the table-creation SQL around the
peer_connections creation) so the runtime schema matches the DB constraint.
🪄 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: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ed1d6aca-21de-4f29-92ce-1d60c4227164
📒 Files selected for processing (3)
public/dashboard.htmlschema.sqlsrc/worker.py
ayesha1145
force-pushed
the
feat/peer-connections
branch
from
124ace8 to
b6c7e16
Compare
Contributor
Author
|
Addressed latest CodeRabbit feedback:
|
Contributor
Author
|
@A1L13N this PR is ready for review — all CodeRabbit conversations resolved ✅ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a peer connections feature allowing students to connect with each other based on shared learning activities.
Changes
schema.sql + src/worker.py DDL:
peer_connectionstable: requester_id, addressee_id, status (pending/accepted/declined), optional message, timestampssrc/worker.py — 5 new API endpoints:
public/dashboard.html:
Security
Peer Connections System
This PR adds a peer connections feature so students can discover and connect with peers who share learning activities, manage incoming requests, and view suggestions.
Database Changes
API Endpoints (authentication required)
Server-side improvements:
Frontend/UI (public/dashboard.html)
Impact