Merged
Conversation
…l massively - Now the ai has more elaborate pool of questions to figure out from
feat(ui/ai): Added more Tool components and expanded the Question Pool massively - Now the ai has more elaborate pool of questions to figure out from
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis pull request introduces 17 new component types (Worker, Container, Gateway, DNS, Firewall, Proxy, NoSQL, Search, GraphDB, PubSub, WebSocket, Logger, Metrics, Tracer, Auth, WAF, Vault) across the canvas and simulation infrastructure. Updates include expanded component palette sections, color mappings for the design canvas, capacity entries for simulation nodes, enhanced question generation with randomized topic selection and additional fallback questions, hydration warning suppressions, and TypeScript compilation notes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
✅ Deploy Preview for system-craft-staging ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/lib/ai/questionGenerator.ts (1)
78-81: Use an unbiased shuffle for topic sampling.
sort(() => Math.random() - 0.5)is biased and can skew topic distribution. Prefer Fisher–Yates for reliable random subsets.♻️ Proposed refactor
function pickRandom<T>(arr: T[], count: number): T[] { - const shuffled = [...arr].sort(() => Math.random() - 0.5); - return shuffled.slice(0, count); + const shuffled = [...arr]; + for (let i = shuffled.length - 1; i > 0; i -= 1) { + const j = Math.floor(Math.random() * (i + 1)); + [shuffled[i], shuffled[j]] = [shuffled[j], shuffled[i]]; + } + return shuffled.slice(0, Math.max(0, count)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/ai/questionGenerator.ts` around lines 78 - 81, The pickRandom function uses a biased shuffle via sort(() => Math.random() - 0.5); replace it with an unbiased Fisher–Yates shuffle: copy the input array (e.g., const result = [...arr]), perform an in-place Fisher–Yates shuffle on result (swap elements using Math.random() across indices), then return result.slice(0, Math.min(count, result.length)); update the pickRandom<T> implementation to use this approach and ensure it safely handles count > arr.length.components/canvas/DesignCanvas.tsx (1)
10-38: Consider a single typed source for node metadata.Lines 10-38 and 41-69 duplicate type keys (color + label), while the same keys are also maintained in
components/canvas/ComponentPalette.tsxandsrc/lib/simulation/constants.ts. A sharedNodeType+ metadata registry would reduce drift/typo risk as new components are added.Also applies to: 41-69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/canvas/DesignCanvas.tsx` around lines 10 - 38, Duplicate node metadata across COLOR_MAP in DesignCanvas.tsx, the palette in ComponentPalette.tsx, and constants in src/lib/simulation/constants.ts should be consolidated into a single typed source to avoid drift; create a shared export (e.g., a NodeType union or enum plus a NodeMetadata registry object) in one module (for example export NodeType and NODE_METADATA from src/lib/simulation/constants.ts), update DesignCanvas to import NODE_METADATA instead of COLOR_MAP, update ComponentPalette to use the same NODE_METADATA and types, and update any code using the old keys to reference NodeType and NODE_METADATA so all color/label/metadata lives in one place and is type-checked across the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@git_status.txt`:
- Around line 1-4: The PR includes a transient local file git_status.txt which
should not be versioned; remove git_status.txt from the commit and from the repo
history by deleting the file from the working tree/commit and ensuring it is
ignored going forward (add the pattern git_status.txt to .gitignore or to your
local exclude file), then amend or create a new commit without the file (or drop
it with git rm --cached if you need to keep a local copy) and force-push or
rebase as appropriate so the PR no longer contains the artifact.
In `@src/lib/ai/questionGenerator.ts`:
- Around line 222-223: Fix the user-facing grammar in the fallback requirement
strings by replacing the incorrect phrase 'Engineers can create, toggles, and
archive feature flags via a dashboard' with 'Engineers can create, toggle, and
archive feature flags via a dashboard' (and apply the same correction to the
duplicate string at the other occurrence identified by the same text); locate
the exact string occurrences in questionGenerator.ts and update them so
generated fallback questions use the corrected wording.
In `@src/lib/firebase/AuthContext.tsx`:
- Line 25: Remove the unnecessary eslint-disable comment inside the AuthContext
component's useEffect: delete the line containing "// eslint-disable-next-line
react-hooks/set-state-in-effect -- intentional: single synchronous set at mount,
not a cascading render" so the file src/lib/firebase/AuthContext.tsx no longer
contains a non-existent rule override; keep the surrounding useEffect and state
update logic unchanged.
In `@src/lib/simulation/constants.ts`:
- Line 10: The DNS constant (and other node capacity constants set to Infinity)
will render as confusing values; replace Infinity in
src/lib/simulation/constants.ts (e.g., DNS and the constants at lines 23-25)
with a large but finite ceiling (for example 1_000_000_000) or a named constant
like MAX_RENDERABLE_CAPACITY, and/or update the DesignCanvas component's
capacity formatting logic (the capacity formatting code in DesignCanvas) to
explicitly detect non-finite values and render a friendly label (e.g.,
"Unlimited" or "∞") instead of numeric formatting; pick one approach and apply
it consistently to the constants (DNS and the other Infinity entries) and the
DesignCanvas capacity formatter.
---
Nitpick comments:
In `@components/canvas/DesignCanvas.tsx`:
- Around line 10-38: Duplicate node metadata across COLOR_MAP in
DesignCanvas.tsx, the palette in ComponentPalette.tsx, and constants in
src/lib/simulation/constants.ts should be consolidated into a single typed
source to avoid drift; create a shared export (e.g., a NodeType union or enum
plus a NodeMetadata registry object) in one module (for example export NodeType
and NODE_METADATA from src/lib/simulation/constants.ts), update DesignCanvas to
import NODE_METADATA instead of COLOR_MAP, update ComponentPalette to use the
same NODE_METADATA and types, and update any code using the old keys to
reference NodeType and NODE_METADATA so all color/label/metadata lives in one
place and is type-checked across the codebase.
In `@src/lib/ai/questionGenerator.ts`:
- Around line 78-81: The pickRandom function uses a biased shuffle via sort(()
=> Math.random() - 0.5); replace it with an unbiased Fisher–Yates shuffle: copy
the input array (e.g., const result = [...arr]), perform an in-place
Fisher–Yates shuffle on result (swap elements using Math.random() across
indices), then return result.slice(0, Math.min(count, result.length)); update
the pickRandom<T> implementation to use this approach and ensure it safely
handles count > arr.length.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b30315a5-f055-48f9-ae67-736d486638b5
📒 Files selected for processing (9)
app/layout.tsxcomponents/canvas/ComponentPalette.tsxcomponents/canvas/DesignCanvas.tsxgit_status.txtsrc/lib/ai/questionGenerator.tssrc/lib/firebase/AuthContext.tsxsrc/lib/simulation/constants.tstsc_output.txttsc_output_2.txt
Comment on lines
+1
to
+4
| M app/practice/[id]/page.tsx | ||
| M src/lib/practice/storage.ts | ||
| ?? git_status.txt | ||
| ?? tsc_output.txt |
Contributor
There was a problem hiding this comment.
Remove local Git status artifacts from the PR.
git_status.txt is transient local-state output and should not be versioned; it becomes stale immediately and adds review noise. Please drop it from the commit (and ignore this pattern if needed).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@git_status.txt` around lines 1 - 4, The PR includes a transient local file
git_status.txt which should not be versioned; remove git_status.txt from the
commit and from the repo history by deleting the file from the working
tree/commit and ensuring it is ignored going forward (add the pattern
git_status.txt to .gitignore or to your local exclude file), then amend or
create a new commit without the file (or drop it with git rm --cached if you
need to keep a local copy) and force-push or rebase as appropriate so the PR no
longer contains the artifact.
Comment on lines
+222
to
+223
| 'Engineers can create, toggles, and archive feature flags via a dashboard', | ||
| 'Flags support percentage rollouts, user segment targeting, and kill switches', |
Contributor
There was a problem hiding this comment.
Fix user-facing grammar in fallback requirements.
Line 222 and Line 261 have copy issues that will surface directly in generated fallback questions.
✏️ Suggested copy fixes
- 'Engineers can create, toggles, and archive feature flags via a dashboard',
+ 'Engineers can create, toggle, and archive feature flags via a dashboard',
...
- 'Scores update reflect within 1 second',
+ 'Score updates are reflected within 1 second',Also applies to: 261-261
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/ai/questionGenerator.ts` around lines 222 - 223, Fix the user-facing
grammar in the fallback requirement strings by replacing the incorrect phrase
'Engineers can create, toggles, and archive feature flags via a dashboard' with
'Engineers can create, toggle, and archive feature flags via a dashboard' (and
apply the same correction to the duplicate string at the other occurrence
identified by the same text); locate the exact string occurrences in
questionGenerator.ts and update them so generated fallback questions use the
corrected wording.
| if (!auth) { | ||
| // eslint-disable-next-line react-hooks/set-state-in-effect | ||
| // No Firebase configured — mark loading as done immediately. | ||
| // eslint-disable-next-line react-hooks/set-state-in-effect -- intentional: single synchronous set at mount, not a cascading render |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if 'react-hooks/set-state-in-effect' is defined in the project's eslint configuration
# Search for eslint configuration files
fd -e js -e json -e yaml -e yml -e cjs -e mjs '(\.eslintrc|eslint\.config)' --exec cat {}
# Search for the specific rule name in the codebase
rg -n 'set-state-in-effect' -g '!*.tsx' -g '!*.ts' -g '!*.jsx' -g '!*.js' --type-add 'config:*.{json,yml,yaml,cjs,mjs}' -t configRepository: Shashank0701-byte/System-Craft
Length of output: 538
Remove the unnecessary eslint-disable comment.
The rule react-hooks/set-state-in-effect is not defined in your eslint configuration and is not a standard rule from eslint-plugin-react-hooks (the standard rules are rules-of-hooks and exhaustive-deps). Setting state synchronously in a useEffect is valid React, so this disable comment is unnecessary and adds noise. Remove the entire comment on line 25.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/firebase/AuthContext.tsx` at line 25, Remove the unnecessary
eslint-disable comment inside the AuthContext component's useEffect: delete the
line containing "// eslint-disable-next-line react-hooks/set-state-in-effect --
intentional: single synchronous set at mount, not a cascading render" so the
file src/lib/firebase/AuthContext.tsx no longer contains a non-existent rule
override; keep the surrounding useEffect and state update logic unchanged.
| Gateway: 50000, // API Gateway handles 50k RPS | ||
| LB: 100000, // Load Balancer handles 100k RPS | ||
| CDN: 500000, // CDN handles 500k RPS (edge cached) | ||
| DNS: Infinity, // DNS resolution is effectively unlimited |
Contributor
There was a problem hiding this comment.
Avoid rendering-facing Infinity capacities for regular node types.
Line 10 and Lines 23-25 introduce Infinity capacities for nodes that are rendered in metrics chips, and components/canvas/DesignCanvas.tsx Line 948 formats capacity numerically. This will show confusing values like Infinityk RPS. Consider finite high ceilings here, or add explicit non-finite formatting in the renderer.
Also applies to: 23-25
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/simulation/constants.ts` at line 10, The DNS constant (and other node
capacity constants set to Infinity) will render as confusing values; replace
Infinity in src/lib/simulation/constants.ts (e.g., DNS and the constants at
lines 23-25) with a large but finite ceiling (for example 1_000_000_000) or a
named constant like MAX_RENDERABLE_CAPACITY, and/or update the DesignCanvas
component's capacity formatting logic (the capacity formatting code in
DesignCanvas) to explicitly detect non-finite values and render a friendly label
(e.g., "Unlimited" or "∞") instead of numeric formatting; pick one approach and
apply it consistently to the constants (DNS and the other Infinity entries) and
the DesignCanvas capacity formatter.
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
1 participant
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.
Summary by CodeRabbit