Skip to content

Feat/inflight request coalescing#401

Open
Priyanshu1-62 wants to merge 3 commits into
Canopus-Labs:mainfrom
Priyanshu1-62:feat/inflight-request-coalescing
Open

Feat/inflight request coalescing#401
Priyanshu1-62 wants to merge 3 commits into
Canopus-Labs:mainfrom
Priyanshu1-62:feat/inflight-request-coalescing

Conversation

@Priyanshu1-62

Copy link
Copy Markdown

📝 Pull Request Description

Related Issue

Summary

  • Implemented in-flight request coalescing for AI-generated aptitude questions to prevent cache stampedes on cache misses.
  • Concurrent requests for the same topic will now share a single Gemini generation request, reducing redundant API calls while preserving the existing caching behavior and response format.
  • The question generation logic was also extracted into a dedicated helper to improve readability and separation of concerns.

Implementation Details

  • Added an in-flight request registry keyed by the normalized cache key to track ongoing question generation requests.
  • Updated the request flow to reuse an existing Promise for concurrent requests targeting the same topic.
  • Removed completed / failed requests from the registry using a finally block so that future cache misses can trigger fresh generation.
  • Refactored error handling so the helper throws enriched errors, while the route handler remains solely responsible for constructing HTTP responses.
  • Added comments and logs wherever necessary.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature
  • ♻️ Refactoring
  • 📝 Documentation update
  • 🎨 UI/UX improvement
  • 🔥 Other(please describe) ______

How Has This Been Tested?

  • npm run test shows no new failed test due to changes made in this PR.
  • Manually confirmed logical equivalence with original code.

Checklist

  • My code follows the project's guidelines
  • I have tested my changes
  • I have updated documentation where necessary
  • I have linked the related issue
  • My changes do not introduce new warnings or errors

Additional Details

  • Label: type: performance, type: feature

Comment thread backend/routes/AptitudeQuestions.js Outdated
let cleanedText = rawText
.replace(/^\s*```json\s*/i, "")
.replace(/^\s*```\s*/i, "")
.replace(/(\s*```\s*)+$/i, "")
@KaranUnique

Copy link
Copy Markdown
Contributor

@Priyanshu1-62 please take a look on github ai suggestion.

@Priyanshu1-62

Copy link
Copy Markdown
Author

Hi @KaranUnique ,

  • GitHub Advanced Security flagged a ReDos vulnerability. The mentioned code already existed with that vulnerability, it predates my PR.
  • I have intentionally kept the PR focused on in-flight request coalescing feature, and made sure to preserve the existing behavior and functionality.
  • We can keep this PR focused on the assigned enhancement and handle the regex separately.
  • After this PR, I will raise a separate issue to address the vulnerability and think for a suitable solution.

Let me know how you would like to proceed.

@KaranUnique

Copy link
Copy Markdown
Contributor

@Priyanshu1-62 However, codeQL is reporting this as a new alert in code introduced by this PR, specifically on the newly added regex at line 102. Please update the regex to resolve the ReDoS warning before we proceed with the merge

@Priyanshu1-62

Copy link
Copy Markdown
Author

Hi @KaranUnique ,
I've replaced the regex with equivalent string operations to avoid the ReDoS warning while preserving the existing behavior. Let me know if any further changes are necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Prevent duplicate Gemini API requests using in-flight request coalescing

3 participants