-
Couldn't load subscription status.
- Fork 0
;;.maybe? #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Domusgpt
wants to merge
11
commits into
shared-core-implementation
Choose a base branch
from
main
base: shared-core-implementation
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
;;.maybe? #7
Conversation
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
feat: Structured outputs and shared core architecture
Please go over these addres the issue if really an issue an delete these
Shared core implementation
This commit addresses several issues in the API to improve its robustness and security:
1. **Rate Limiting for Anonymous Users:**
- I replaced the in-memory rate limiter for anonymous users with a Firestore-based solution in `rateLimitMiddleware.ts`.
- Anonymous user requests are now tracked per IP per minute in the `anonymousRateLimits` Firestore collection using transactions, providing better scalability and persistence.
- I added unit tests to `rateLimitMiddleware.test.ts` to verify this new logic, including timer-based reset functionality.
2. **Improved Error Handling for Malformed JSON:**
- I enhanced `parseRoutes.ts` to provide more specific error responses when backend services (Architect or Extractor) return malformed JSON.
- Instead of a generic 500 error, the API now returns a 422 (Unprocessable Entity) error with a clearer message (e.g., "Failed to parse response from Architect service.").
- In development mode, the error response includes a `details` field with the original error and raw response for easier debugging.
- I added unit tests in `parseRoutes.test.ts` to cover these scenarios.
3. **Input Size Limits:**
- I implemented an input size limit of 1MB for `inputData` in `parseRoutes.ts`.
- If `inputData` exceeds this limit, the API returns a 413 (Payload Too Large) error, including the received payload size in the message.
- I added a check to ensure `inputData` is a string, returning a 400 error if not.
- I added unit tests for these checks to `parseRoutes.test.ts`.
4. **Dependency Risk (`eslint-scope`):**
- The issue report mentioned `eslint-scope` as a potential dependency risk. This is a development dependency and does not affect the runtime behavior of the API. No direct changes were made related to this, but it's noted for future review of development dependencies.
These changes significantly improve the API's resilience to malformed inputs, provide clearer error feedback, and implement more robust rate limiting for anonymous users.
Fix: Enhance API security, error handling, and rate limiting
… done so far and provide feedback for Jules to continue.
Jules was unable to complete the task in time. Please review the work…
…ness
This commit implements fixes for several critical security issues identified:
1. **Rate Limiting:**
* Resolved issues where rate limiting was ineffective, particularly for anonymous users.
* Implemented comprehensive RPM (Requests Per Minute), daily, and monthly limits for both anonymous (IP-based) and authenticated users in the main API entry point (`packages/api/src/index.ts`).
* Updated the Express middleware (`packages/api/src/middleware/rateLimitMiddleware.ts`) to mirror these robust limits.
* Instituted a "fail-closed" policy for rate limit checks: if the backend (Firestore) encounters an error during a check, the request is denied, preventing potential abuse during backend issues.
2. **Input Validation and Sanitization (XSS & Server Stability):**
* Sanitized API key names provided via `/v1/user/keys`. HTML special characters and backticks are now encoded before storage and in API responses, mitigating reflected and potential stored XSS vulnerabilities.
* Hardened the `/v1/parse` endpoint by escaping backtick characters in user-supplied `inputData`. This prevents malformed AI prompts that previously led to server errors.
3. **Error Handling Information Leaks:**
* Modified the error response for the `/v1/parse` endpoint. Instead of returning raw internal error messages, a generic message is now sent to the client, while detailed error information is logged server-side. This prevents leakage of potentially sensitive internal state.
4. **Testing:**
* Added a new test suite (`packages/api/src/__tests__/`) with integration tests.
* Rate limiting logic in `packages/api/src/index.ts` is extensively tested, including RPM, daily, monthly limits for anonymous and authenticated users, and fail-closed behavior using Firestore mocks.
* Input sanitization for API key names and backtick escaping for `inputData` are tested, verifying correct output and prompt construction with mocked AI responses.
These changes significantly improve the security posture and reliability of the API.
This commit introduces an intelligent caching layer for Architect searchPlans
to improve performance and reduce redundant AI calls, along with a
self-correction mechanism driven by Extractor performance.
Key changes:
1. **In-Memory LRU Cache for Architect Plans:**
* I implemented an in-memory LRU (Least Recently Used) cache (`Map`-based)
in `packages/api/src/index.ts` to store generated `searchPlan`s.
* Cache keys are generated using a SHA256 hash of the `outputSchema`.
* A `MAX_CACHE_SIZE` is defined to prevent unbounded memory use.
2. **Caching Logic in `/v1/parse` Endpoint:**
* The endpoint now attempts to retrieve a `searchPlan` from the cache
before calling the Architect model.
* If a plan is found (cache hit), the Architect call is skipped.
* Newly generated plans are stored in the cache.
* A `forceRefreshArchitect: true` request body parameter allows you
to bypass the cache and force a new Architect invocation.
3. **Extractor-Driven Re-architecture (Self-Correction):**
* After the Extractor processes data using a cached `searchPlan`, I
evaluate the `parsedData`.
* If a significant number of top-level fields (defined in the
`outputSchema`) are missing or null, the cached `searchPlan` is
considered suboptimal and is automatically invalidated (removed from
the cache).
* This ensures that on subsequent requests with the same `outputSchema`,
the Architect will be re-invoked to generate a potentially better plan.
4. **Response Metadata Enhancements:**
* The API response for `/v1/parse` now includes a `cacheInfo` object in
the `metadata`, indicating:
* `retrievedFromCache: boolean` (if the plan was from cache)
* `invalidatedByExtractor: boolean` (if the plan was invalidated
in the current request due to poor Extractor results).
5. **Architect Prompt Refinement (Minor):**
* I added a subtle instruction to the Architect prompt to encourage the
creation of robust plans that can handle minor input variations.
6. **Comprehensive Testing:**
* I added a new test suite (`index.caching.test.ts`) with extensive
tests for the caching and re-architecture logic.
* The tests cover cache hits, misses, `forceRefreshArchitect`, LRU eviction,
and the Extractor failure invalidation mechanism, ensuring the new
system behaves as expected under various conditions.
These changes aim to make the Parserator API more efficient, adaptive, and
cost-effective by intelligently reusing Architect computations while also
allowing the system to learn from and correct suboptimal parsing strategies.
This commit improves the Architect `searchPlan` caching mechanism by
incorporating a fingerprint of the input data sample into the cache key.
This makes the cache more granular, ensuring that plans are reused only
when both the output schema and the structural characteristics of the
input data are similar.
Key changes:
1. **Input Data Fingerprinting Strategy:**
* Designed and implemented a `generateInputFingerprint` function in
`packages/api/src/index.ts`.
* The fingerprint is generated from the first 1KB of the input data
sample and includes heuristics such as:
* Presence of JSON/XML characters.
* Number of lines and average line length.
* Colon count.
* Numeric density.
* This provides a lightweight yet effective way to differentiate input
structures.
2. **Compound Cache Key:**
* Modified `generateCacheKey` to combine a hash of the `outputSchema`
with the newly generated `inputFingerprint`.
* This more specific cache key is now used for all cache operations
(get, set, delete) in the `/v1/parse` endpoint.
3. **Integration into `/v1/parse`:**
* The `inputFingerprint` is generated from the input sample.
* The compound cache key is used for looking up cached plans, storing
new plans, and invalidating plans via the Extractor-driven
re-architecture logic.
4. **Updated Tests:**
* Added direct unit tests for the `generateInputFingerprint` function
to validate its behavior with various input types.
* Updated all existing caching tests in
`packages/api/src/__tests__/index.caching.test.ts` to reflect the
new compound cache key logic.
* Added new test scenarios to specifically verify that changes in input
data (leading to different fingerprints) correctly result in cache
misses, even if the output schema remains the same.
This enhancement aims to increase cache effectiveness, reduce unnecessary
Architect invocations when input structures vary for the same schema, and
further improve the performance and cost-efficiency of the Parserator API.
The existing Extractor-driven re-architecture mechanism remains compatible
and will now operate on these more granular cache entries.
Jules wip 3583282160126608178
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Pull Request
📋 Description
🔗 Related Issue
Fixes #(issue_number)
🛠️ Type of Change
🧪 Testing
Test Details:
🔍 EMA Compliance Check
📝 Changes Made
📱 Component Impact
🔒 Breaking Changes
None / Describe breaking changes here
📸 Screenshots/Examples
✅ Checklist
🎯 Additional Notes
By submitting this PR, I confirm that: