Josh dev#34
Conversation
…s; update AnalyzeHandler to consolidate error and annotation responses into single arrays.
- Node port redesign - HMR for backend - Added new nodes - Node defaults
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
napkin-app/src/App.tsx (1)
82-111:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle non-2xx analyze responses before parsing JSON.
AnalyzeHandlerstill emits plain-text errors viahttp.Error(...)for invalid input, sores.json()will throw and the user loses the actual backend message. Checkres.okfirst and surface the response text as anAnalyzeError.Proposed fix
- const data = await res.json(); + if (!res.ok) { + const message = await res.text(); + return [ + { + severity: "error", + message: message || `Analyze failed (${res.status})`, + }, + ]; + } + + const data = await res.json();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@napkin-app/src/App.tsx` around lines 82 - 111, The code calls res.json() unconditionally which throws for non-2xx text errors from AnalyzeHandler; before parsing, check res.ok and if false await res.text() and push that text into menuError (e.g., menuError.push({ severity: "error", message: text })) then return menuError; only call await res.json() and process data.errors into newNodeErrors/menuError when res.ok is true, and still call setNodeErrors(newNodeErrors) where appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@napkin-app/src/components/FloatingMenu.tsx`:
- Around line 101-111: When handling a successful compile in the FloatingMenu
component (the branch that calls setCompiledOutput and setCopyState), also clear
any stale errors by calling setCompileErrors([]) and setErrorsVisible(false) so
previous compileErrors don't remain visible; update the same success branch
where setCompiledOutput({ output: data.output, target: data.target ??
compileTarget }) is set to also reset compileErrors and errorsVisible.
- Around line 122-124: The code casts a thrown value to Error when calling
setCompileErrors, which can break if non-Error values are thrown; update the
handler around setCompileErrors to safely extract the message (e.g., check "err
instanceof Error" and use err.message, otherwise use String(err) or a fallback)
so the error entry always has a reliable message; modify the block that builds
the array passed to setCompileErrors (the site that currently does "{ severity:
'error', message: (err as Error).message }") to use this safe extraction.
In `@napkin-app/src/lib/apiBase.ts`:
- Around line 3-6: The code validates raw.trim() but returns the untrimmed raw,
so normalize by trimming before returning: assign const trimmed = raw.trim() (or
reuse raw = raw.trim()), then apply and return trimmed.replace(/\/$/, "");
update the logic around the variable raw (or use a new trimmed variable) so
getBase URL always returns the trimmed-and-trailing-slash-removed value.
In `@napkin-app/src/lib/transformer/transformer.ts`:
- Around line 43-47: getKind(data.kind) can return undefined causing
terraformType to be missing; add an explicit guard after const def =
getKind(data.kind) that checks if def is falsy and then logs a clear
warning/error (including data.kind) and provides a safe fallback (e.g. set
defLabel/terraformType from data.spec if present or a default like "unknown" /
short-circuit return of an error node) before computing label and terraformType
so downstream compile validation won't receive nodes without a type; update
references in this function to use the guarded/normalized def when building the
node payload.
In `@napkin-app/src/ToolBox.tsx`:
- Around line 15-24: Replace the clickable non-semantic div used for each
toolbox item with a real button element to restore keyboard accessibility:
change the element that currently uses key={node.kind} and onClick={() =>
onAdd(node.kind)} into a <button type="button"> that preserves the same
className, key, children (node.label and node.description) and still calls
onAdd(node.kind) on activation; ensure any styling/ARIA needed for
presentation/role remains and remove cursor-pointer if redundant so keyboard and
screen reader users can focus/activate items.
In `@napkin-backend/compiler/tf_target.go`:
- Around line 439-451: napkinLBTargets can append the same GraphNode multiple
times causing duplicate LB->EC2 attachments; update napkinLBTargets to
deduplicate targets per LB by tracking seen EC2 IDs (e.g., use a temporary
map[string]struct{} or map[string]bool keyed by to.ID) for each from.ID and only
append to out[from.ID] when the EC2 ID hasn't been seen yet; keep existing
checks (from.Type == "aws_lb", to.Type == "aws_instance", and edgeAllowsLBToEC2)
and ensure the function returns the same map[string][]GraphNode shape but with
duplicates removed.
- Around line 157-166: The ALB follow-up resources use lb.LocalName directly
which can be empty; compute a canonical name variable (e.g., localName :=
lb.LocalName; if localName == "" { localName = lb.ID }) and use that canonical
localName when building tgName, listenerName, tgPrefix usages and when calling
napkinLBTargetGroupBlock and napkinLBListenerBlock (and any other references in
the same block/loop later on) so they match the earlier fallback behavior and
avoid broken references.
In `@napkin-backend/graph/intent_graph.go`:
- Around line 84-89: Normalization currently only checks for key existence in
specMap and leaves empty-string values untouched; update the logic around
specMap to treat blank values as missing by checking for empty/whitespace (e.g.,
if val == "" or strings.TrimSpace(val) == "") and then assign defaults: set
specMap["class"] = "resource" when blank and set specMap["type"] =
def.TerraformType when blank. Modify the same block that references specMap and
def.TerraformType so empty-string entries are overridden with the defaults.
- Around line 26-44: The FromJSON loader currently only initializes ig.Nodes
when nil, which can leave stale data when reusing an IntentGraph; update the
FromJSON implementation (method on IntentGraph that unmarshals into InnerGraph)
to unconditionally reset the graph state before loading: set ig.Nodes =
make(map[NodeID]Node) (instead of the nil-check), clear ig.Edges (e.g., set to
nil or an empty slice/map) and then unmarshal payload into InnerGraph and
populate ig.Region, ig.Nodes and ig.Edges from payload as before so no old
nodes/edges remain.
In `@napkin-backend/lib/handlers/analyze.go`:
- Around line 58-64: The handler is always merging both analyzers; update the
logic that builds allErrors and allAnnotations to honor AnalyzeRequest.Type
(e.g., check req.Type or r.Type) so that when the request asks for only Network
or only Performance analysis you append only networkErrors/networkAnnotations or
only performanceErrors/performanceAnnotations respectively; likewise adjust the
code that builds the response (the section around where results are returned) to
return only the chosen analyzer's results when Type is specified and fall back
to merging both only when Type indicates "Both" or is unspecified.
In `@napkin-backend/lib/handlers/compile_helpers.go`:
- Around line 37-40: The code currently uses fmt.Sprint(specMap["type"]) which
turns a missing spec.type into the string "<nil>" and bypasses the empty check;
update the logic in compile_helpers.go to first check the presence and non-nil
value of specMap["type"] (e.g., retrieve val, ok := specMap["type"] and ensure
val != nil) and return fmt.Errorf("node %s missing type in spec", node.ID) if
absent, then only afterwards convert/trim the value into tfType (or assert it to
string) so tfType cannot be "<nil>".
In `@package.json`:
- Line 9: The dev:backend npm script currently invokes air via a hardcoded POSIX
command substitution ("$(go env GOPATH)/bin/air") which breaks on non-POSIX
shells; update the dev:backend script to call the air executable directly (use
"air") so it relies on PATH and works across shells, ensuring users have
installed air and added its install directory to PATH per Air's setup
instructions.
---
Outside diff comments:
In `@napkin-app/src/App.tsx`:
- Around line 82-111: The code calls res.json() unconditionally which throws for
non-2xx text errors from AnalyzeHandler; before parsing, check res.ok and if
false await res.text() and push that text into menuError (e.g., menuError.push({
severity: "error", message: text })) then return menuError; only call await
res.json() and process data.errors into newNodeErrors/menuError when res.ok is
true, and still call setNodeErrors(newNodeErrors) where appropriate.
🪄 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: 6e58e3ed-cc06-47bb-89e4-40ebe77e5e1a
⛔ Files ignored due to path filters (1)
napkin-app/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (32)
.gitignorenapkin-app/components.jsonnapkin-app/public/graph.jsonnapkin-app/src/App.cssnapkin-app/src/App.tsxnapkin-app/src/ToolBox.tsxnapkin-app/src/components/FloatingMenu.tsxnapkin-app/src/components/ResourceNode.tsxnapkin-app/src/components/ui/dialog.tsxnapkin-app/src/components/ui/napkin.code-workspacenapkin-app/src/index.cssnapkin-app/src/lib/apiBase.tsnapkin-app/src/lib/kinds.tsnapkin-app/src/lib/transformer/Handler.tsxnapkin-app/src/lib/transformer/transformer.tsnapkin-backend/.air.tomlnapkin-backend/analysis/analyzer.gonapkin-backend/compiler/target.gonapkin-backend/compiler/tf_target.gonapkin-backend/compiler/tf_target_test.gonapkin-backend/compiler/tf_writer.gonapkin-backend/graph/graph_test.gonapkin-backend/graph/intent_graph.gonapkin-backend/graph/intent_graph_test.gonapkin-backend/graph/kinds.gonapkin-backend/graph/type.gonapkin-backend/lib/handlers/analyze.gonapkin-backend/lib/handlers/compile.gonapkin-backend/lib/handlers/compile_helpers.gonapkin-backend/lib/handlers/compile_helpers_test.gonapkin-backend/main.gopackage.json
💤 Files with no reviewable changes (3)
- .gitignore
- napkin-app/src/components/ui/napkin.code-workspace
- napkin-app/src/lib/transformer/Handler.tsx
| } else if ( | ||
| data.success && | ||
| typeof data.output === "string" && | ||
| data.output.length > 0 | ||
| ) { | ||
| setCompiledOutput({ | ||
| output: data.output, | ||
| target: data.target ?? compileTarget, | ||
| }); | ||
| setCopyState("idle"); | ||
| } else { |
There was a problem hiding this comment.
Clear stale compile errors on successful compile.
On Line 106 success sets compiledOutput, but previous compileErrors/errorsVisible are not cleared, so old errors can remain visible after a successful compile.
Suggested fix
} else if (
data.success &&
typeof data.output === "string" &&
data.output.length > 0
) {
+ setCompileErrors([]);
+ setErrorsVisible(false);
setCompiledOutput({
output: data.output,
target: data.target ?? compileTarget,
});
setCopyState("idle");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else if ( | |
| data.success && | |
| typeof data.output === "string" && | |
| data.output.length > 0 | |
| ) { | |
| setCompiledOutput({ | |
| output: data.output, | |
| target: data.target ?? compileTarget, | |
| }); | |
| setCopyState("idle"); | |
| } else { | |
| } else if ( | |
| data.success && | |
| typeof data.output === "string" && | |
| data.output.length > 0 | |
| ) { | |
| setCompileErrors([]); | |
| setErrorsVisible(false); | |
| setCompiledOutput({ | |
| output: data.output, | |
| target: data.target ?? compileTarget, | |
| }); | |
| setCopyState("idle"); | |
| } else { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-app/src/components/FloatingMenu.tsx` around lines 101 - 111, When
handling a successful compile in the FloatingMenu component (the branch that
calls setCompiledOutput and setCopyState), also clear any stale errors by
calling setCompileErrors([]) and setErrorsVisible(false) so previous
compileErrors don't remain visible; update the same success branch where
setCompiledOutput({ output: data.output, target: data.target ?? compileTarget })
is set to also reset compileErrors and errorsVisible.
| setCompileErrors([ | ||
| { severity: "error", message: (err as Error).message }, | ||
| ]); |
There was a problem hiding this comment.
Handle non-Error throw values safely.
Line 123 casts err to Error. If something else is thrown, the message becomes unreliable.
Suggested fix
} catch (err) {
+ const message =
+ err instanceof Error ? err.message : "Compile request failed";
setCompileErrors([
- { severity: "error", message: (err as Error).message },
+ { severity: "error", message },
]);
setErrorsVisible(true);
setCompiledOutput(null);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setCompileErrors([ | |
| { severity: "error", message: (err as Error).message }, | |
| ]); | |
| const message = | |
| err instanceof Error ? err.message : "Compile request failed"; | |
| setCompileErrors([ | |
| { severity: "error", message }, | |
| ]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-app/src/components/FloatingMenu.tsx` around lines 122 - 124, The code
casts a thrown value to Error when calling setCompileErrors, which can break if
non-Error values are thrown; update the handler around setCompileErrors to
safely extract the message (e.g., check "err instanceof Error" and use
err.message, otherwise use String(err) or a fallback) so the error entry always
has a reliable message; modify the block that builds the array passed to
setCompileErrors (the site that currently does "{ severity: 'error', message:
(err as Error).message }") to use this safe extraction.
| const raw = import.meta.env.VITE_API_BASE; | ||
| if (typeof raw === "string" && raw.trim()) { | ||
| return raw.replace(/\/$/, ""); | ||
| } |
There was a problem hiding this comment.
Normalize VITE_API_BASE before returning it.
At Line 4, you validate raw.trim() but return the untrimmed raw on Line 5. Values with leading/trailing spaces produce malformed URLs.
✅ Suggested fix
export function apiBase(): string {
const raw = import.meta.env.VITE_API_BASE;
- if (typeof raw === "string" && raw.trim()) {
- return raw.replace(/\/$/, "");
+ if (typeof raw === "string") {
+ const normalized = raw.trim().replace(/\/+$/, "");
+ if (normalized) return normalized;
}
return "http://localhost:8080";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const raw = import.meta.env.VITE_API_BASE; | |
| if (typeof raw === "string" && raw.trim()) { | |
| return raw.replace(/\/$/, ""); | |
| } | |
| const raw = import.meta.env.VITE_API_BASE; | |
| if (typeof raw === "string") { | |
| const normalized = raw.trim().replace(/\/+$/, ""); | |
| if (normalized) return normalized; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-app/src/lib/apiBase.ts` around lines 3 - 6, The code validates
raw.trim() but returns the untrimmed raw, so normalize by trimming before
returning: assign const trimmed = raw.trim() (or reuse raw = raw.trim()), then
apply and return trimmed.replace(/\/$/, ""); update the logic around the
variable raw (or use a new trimmed variable) so getBase URL always returns the
trimmed-and-trailing-slash-removed value.
| const def = getKind(data.kind); | ||
|
|
||
| const label = data.spec?.label ?? def?.label ?? ""; | ||
| const terraformType = def?.terraformType ?? data.spec?.terraformType; | ||
|
|
There was a problem hiding this comment.
Unknown kind can silently emit nodes without spec.type.
On Line 43–Line 47, getKind returning undefined can produce a node with no Terraform type, which then fails downstream compile validation. Add an explicit guard/log+fallback before building the node payload.
Suggested fix
- const def = getKind(data.kind);
-
- const label = data.spec?.label ?? def?.label ?? "";
- const terraformType = def?.terraformType ?? data.spec?.terraformType;
+ const def = getKind(data.kind);
+ const terraformType = data.spec?.terraformType ?? def?.terraformType;
+ if (data.kind && !def && !terraformType) {
+ console.warn(`Unknown kind "${data.kind}" for node "${node.id}"`);
+ return;
+ }
+ const label = data.spec?.label ?? def?.label ?? "";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const def = getKind(data.kind); | |
| const label = data.spec?.label ?? def?.label ?? ""; | |
| const terraformType = def?.terraformType ?? data.spec?.terraformType; | |
| const def = getKind(data.kind); | |
| const terraformType = data.spec?.terraformType ?? def?.terraformType; | |
| if (data.kind && !def && !terraformType) { | |
| console.warn(`Unknown kind "${data.kind}" for node "${node.id}"`); | |
| return; | |
| } | |
| const label = data.spec?.label ?? def?.label ?? ""; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-app/src/lib/transformer/transformer.ts` around lines 43 - 47,
getKind(data.kind) can return undefined causing terraformType to be missing; add
an explicit guard after const def = getKind(data.kind) that checks if def is
falsy and then logs a clear warning/error (including data.kind) and provides a
safe fallback (e.g. set defLabel/terraformType from data.spec if present or a
default like "unknown" / short-circuit return of an error node) before computing
label and terraformType so downstream compile validation won't receive nodes
without a type; update references in this function to use the guarded/normalized
def when building the node payload.
| <div | ||
| key={node.kind} | ||
| onClick={() => onAdd(node.kind)} | ||
| className="flex flex-col px-3 py-2 rounded-lg border border-gray-200 bg-gray-50 cursor-pointer hover:border-gray-400 hover:bg-white transition-all duration-150 select-none" | ||
| > | ||
| <span className="text-sm font-medium text-gray-700">{node.label}</span> | ||
| <span className="text-sm font-medium text-gray-700"> | ||
| {node.label} | ||
| </span> | ||
| <span className="text-[11px] text-gray-400">{node.description}</span> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Interactive non-semantic elements in ToolBox.tsx:"
rg -nP '<div[^>]*onClick=' napkin-app/src/ToolBox.tsx || true
echo
echo "Keyboard accessibility attributes/handlers present?"
rg -n 'role="button"|tabIndex=|onKeyDown=|onKeyUp=' napkin-app/src/ToolBox.tsx || trueRepository: acmutsa/napkin
Length of output: 161
🏁 Script executed:
# Check if the file exists and read the relevant lines
if [ -f "napkin-app/src/ToolBox.tsx" ]; then
echo "=== File exists. Reading lines 15-24 ==="
sed -n '15,24p' napkin-app/src/ToolBox.tsx
else
echo "File not found at napkin-app/src/ToolBox.tsx"
# Try to find ToolBox files
echo ""
echo "=== Searching for ToolBox files ==="
find . -name "*ToolBox*" -type f 2>/dev/null || true
fiRepository: acmutsa/napkin
Length of output: 575
Use a semantic button for toolbox items (keyboard accessibility).
The clickable <div> at lines 15–24 lacks keyboard accessibility and prevents keyboard-only users from inserting nodes. Convert it to a <button type="button"> element to enable focus and Enter/Space key activation.
♿ Suggested change
- <div
+ <button
+ type="button"
key={node.kind}
onClick={() => onAdd(node.kind)}
- className="flex flex-col px-3 py-2 rounded-lg border border-gray-200 bg-gray-50 cursor-pointer hover:border-gray-400 hover:bg-white transition-all duration-150 select-none"
+ className="flex w-full text-left flex-col px-3 py-2 rounded-lg border border-gray-200 bg-gray-50 cursor-pointer hover:border-gray-400 hover:bg-white transition-all duration-150 select-none"
>
<span className="text-sm font-medium text-gray-700">
{node.label}
</span>
<span className="text-[11px] text-gray-400">{node.description}</span>
- </div>
+ </button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div | |
| key={node.kind} | |
| onClick={() => onAdd(node.kind)} | |
| className="flex flex-col px-3 py-2 rounded-lg border border-gray-200 bg-gray-50 cursor-pointer hover:border-gray-400 hover:bg-white transition-all duration-150 select-none" | |
| > | |
| <span className="text-sm font-medium text-gray-700">{node.label}</span> | |
| <span className="text-sm font-medium text-gray-700"> | |
| {node.label} | |
| </span> | |
| <span className="text-[11px] text-gray-400">{node.description}</span> | |
| </div> | |
| <button | |
| type="button" | |
| key={node.kind} | |
| onClick={() => onAdd(node.kind)} | |
| className="flex w-full text-left flex-col px-3 py-2 rounded-lg border border-gray-200 bg-gray-50 cursor-pointer hover:border-gray-400 hover:bg-white transition-all duration-150 select-none" | |
| > | |
| <span className="text-sm font-medium text-gray-700"> | |
| {node.label} | |
| </span> | |
| <span className="text-[11px] text-gray-400">{node.description}</span> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-app/src/ToolBox.tsx` around lines 15 - 24, Replace the clickable
non-semantic div used for each toolbox item with a real button element to
restore keyboard accessibility: change the element that currently uses
key={node.kind} and onClick={() => onAdd(node.kind)} into a <button
type="button"> that preserves the same className, key, children (node.label and
node.description) and still calls onAdd(node.kind) on activation; ensure any
styling/ARIA needed for presentation/role remains and remove cursor-pointer if
redundant so keyboard and screen reader users can focus/activate items.
| if ig.Nodes == nil { | ||
| ig.Nodes = make(map[NodeID]Node) | ||
| } | ||
|
|
||
| var payload InnerGraph // Change this from GraphJSON | ||
| if err := json.Unmarshal(data, &payload); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| ig.Region = payload.Region | ||
|
|
||
| for _, nodeSlice := range payload.Nodes { | ||
| for _, n := range nodeSlice { | ||
| ig.Nodes[n.ID] = n | ||
| } | ||
| } | ||
|
|
||
| ig.Edges = payload.Edges | ||
| return nil |
There was a problem hiding this comment.
Reset graph state in FromJSON before loading a new payload.
Current logic only initializes ig.Nodes when nil. Reusing the same IntentGraph instance can keep stale nodes from prior payloads.
Suggested fix
func (ig *IntentGraph) FromJSON(data []byte) error {
- if ig.Nodes == nil {
- ig.Nodes = make(map[NodeID]Node)
- }
+ ig.Nodes = make(map[NodeID]Node)
+ ig.Edges = nil
@@
for _, nodeSlice := range payload.Nodes {
for _, n := range nodeSlice {
ig.Nodes[n.ID] = n
}
}
ig.Edges = payload.Edges
return nil
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-backend/graph/intent_graph.go` around lines 26 - 44, The FromJSON
loader currently only initializes ig.Nodes when nil, which can leave stale data
when reusing an IntentGraph; update the FromJSON implementation (method on
IntentGraph that unmarshals into InnerGraph) to unconditionally reset the graph
state before loading: set ig.Nodes = make(map[NodeID]Node) (instead of the
nil-check), clear ig.Edges (e.g., set to nil or an empty slice/map) and then
unmarshal payload into InnerGraph and populate ig.Region, ig.Nodes and ig.Edges
from payload as before so no old nodes/edges remain.
| if _, exists := specMap["class"]; !exists { | ||
| specMap["class"] = "resource" | ||
| } | ||
| if _, exists := specMap["type"]; !exists { | ||
| specMap["type"] = def.TerraformType | ||
| } |
There was a problem hiding this comment.
Treat blank spec.class / spec.type as missing during normalization.
On Line 84 and Line 87, only key existence is checked. Empty-string values currently skip defaulting and can propagate invalid metadata.
Suggested fix
- if _, exists := specMap["class"]; !exists {
+ if cls, ok := specMap["class"].(string); !ok || strings.TrimSpace(cls) == "" {
specMap["class"] = "resource"
}
- if _, exists := specMap["type"]; !exists {
+ if typ, ok := specMap["type"].(string); !ok || strings.TrimSpace(typ) == "" {
specMap["type"] = def.TerraformType
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-backend/graph/intent_graph.go` around lines 84 - 89, Normalization
currently only checks for key existence in specMap and leaves empty-string
values untouched; update the logic around specMap to treat blank values as
missing by checking for empty/whitespace (e.g., if val == "" or
strings.TrimSpace(val) == "") and then assign defaults: set specMap["class"] =
"resource" when blank and set specMap["type"] = def.TerraformType when blank.
Modify the same block that references specMap and def.TerraformType so
empty-string entries are overridden with the defaults.
| allErrors := make([]analysis.AnalysisError, 0, len(networkErrors)+len(performanceErrors)) | ||
| allErrors = append(allErrors, networkErrors...) | ||
| allErrors = append(allErrors, performanceErrors...) | ||
|
|
||
| allAnnotations := make([]analysis.Annotation, 0, len(networkAnnotations)+len(performanceAnnotations)) | ||
| allAnnotations = append(allAnnotations, networkAnnotations...) | ||
| allAnnotations = append(allAnnotations, performanceAnnotations...) |
There was a problem hiding this comment.
Honor AnalyzeRequest.Type before returning merged results.
Lines 58–64 always append both analyzers, so selecting a specific analysis type has no effect on backend output.
Suggested direction
+ // example shape
+ var allErrors []analysis.AnalysisError
+ var allAnnotations []analysis.Annotation
+ switch strings.ToLower(req.Type) {
+ case "network", "security":
+ allErrors = append(allErrors, networkErrors...)
+ allAnnotations = append(allAnnotations, networkAnnotations...)
+ case "performance":
+ allErrors = append(allErrors, performanceErrors...)
+ allAnnotations = append(allAnnotations, performanceAnnotations...)
+ default:
+ allErrors = append(allErrors, networkErrors...)
+ allErrors = append(allErrors, performanceErrors...)
+ allAnnotations = append(allAnnotations, networkAnnotations...)
+ allAnnotations = append(allAnnotations, performanceAnnotations...)
+ }Also applies to: 68-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-backend/lib/handlers/analyze.go` around lines 58 - 64, The handler is
always merging both analyzers; update the logic that builds allErrors and
allAnnotations to honor AnalyzeRequest.Type (e.g., check req.Type or r.Type) so
that when the request asks for only Network or only Performance analysis you
append only networkErrors/networkAnnotations or only
performanceErrors/performanceAnnotations respectively; likewise adjust the code
that builds the response (the section around where results are returned) to
return only the chosen analyzer's results when Type is specified and fall back
to merging both only when Type indicates "Both" or is unspecified.
| tfType := strings.TrimSpace(fmt.Sprint(specMap["type"])) | ||
| if tfType == "" { | ||
| return compiler.IR{}, fmt.Errorf("node %s missing type in spec", node.ID) | ||
| } |
There was a problem hiding this comment.
Fail fast when spec.type is missing instead of accepting "<nil>".
On Line 37, fmt.Sprint(specMap["type"]) turns a missing value into "<nil>", so Line 38 never triggers and invalid IR is emitted.
Suggested fix
- tfType := strings.TrimSpace(fmt.Sprint(specMap["type"]))
- if tfType == "" {
- return compiler.IR{}, fmt.Errorf("node %s missing type in spec", node.ID)
- }
+ typeVal, exists := specMap["type"]
+ if !exists {
+ return compiler.IR{}, fmt.Errorf("node %s missing type in spec", node.ID)
+ }
+ typeStr, ok := typeVal.(string)
+ if !ok {
+ return compiler.IR{}, fmt.Errorf("node %s type must be a string", node.ID)
+ }
+ tfType := strings.TrimSpace(typeStr)
+ if tfType == "" {
+ return compiler.IR{}, fmt.Errorf("node %s missing type in spec", node.ID)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tfType := strings.TrimSpace(fmt.Sprint(specMap["type"])) | |
| if tfType == "" { | |
| return compiler.IR{}, fmt.Errorf("node %s missing type in spec", node.ID) | |
| } | |
| typeVal, exists := specMap["type"] | |
| if !exists { | |
| return compiler.IR{}, fmt.Errorf("node %s missing type in spec", node.ID) | |
| } | |
| typeStr, ok := typeVal.(string) | |
| if !ok { | |
| return compiler.IR{}, fmt.Errorf("node %s type must be a string", node.ID) | |
| } | |
| tfType := strings.TrimSpace(typeStr) | |
| if tfType == "" { | |
| return compiler.IR{}, fmt.Errorf("node %s missing type in spec", node.ID) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@napkin-backend/lib/handlers/compile_helpers.go` around lines 37 - 40, The
code currently uses fmt.Sprint(specMap["type"]) which turns a missing spec.type
into the string "<nil>" and bypasses the empty check; update the logic in
compile_helpers.go to first check the presence and non-nil value of
specMap["type"] (e.g., retrieve val, ok := specMap["type"] and ensure val !=
nil) and return fmt.Errorf("node %s missing type in spec", node.ID) if absent,
then only afterwards convert/trim the value into tfType (or assert it to string)
so tfType cannot be "<nil>".
| "dev": "concurrently -n frontend,backend -c cyan,green \"pnpm dev:frontend\" \"pnpm dev:backend\"", | ||
| "dev:frontend": "pnpm --filter napkin-app dev", | ||
| "dev:backend": "cd napkin-backend && go run main.go", | ||
| "dev:backend": "cd napkin-backend && \"$(go env GOPATH)/bin/air\"", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Current dev:backend script:"
jq -r '.scripts["dev:backend"]' package.json
echo
echo "Check if script uses POSIX command substitution (\$()):"
jq -r '.scripts["dev:backend"]' package.json | rg -n '\$\(' || true
echo
echo "Check whether docs mention Air/PATH setup:"
fd -a -i 'readme*' | xargs -r rg -n 'air|dev:backend|GOPATH|GOBIN|PATH'Repository: acmutsa/napkin
Length of output: 350
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Looking for go.mod or air configuration ==="
find . -maxdepth 3 -name "go.mod" -o -name ".air.toml" -o -name "air.toml" | head -20
echo
echo "=== Checking napkin-backend directory structure ==="
ls -la napkin-backend/ | head -20
echo
echo "=== Looking for any setup or installation docs ==="
fd -a -i 'setup|install|contributing' | xargs -r head -30
echo
echo "=== Check if air is referenced in go.mod ==="
if [ -f napkin-backend/go.mod ]; then
grep -i 'air' napkin-backend/go.mod || echo "air not found in go.mod"
fiRepository: acmutsa/napkin
Length of output: 998
🌐 Web query:
air golang tool installation methods GOPATH vs PATH
💡 Result:
Air is a live-reload tool for Go applications (github.com/air-verse/air). Its installation methods involve go install, which places the binary in
Citations:
- 1: https://pkg.go.dev/github.com/air-verse/air
- 2: https://github.com/air-verse/air
- 3: No .air folder after installing with go install with Go 1.24.3 air-verse/air#770
- 4: Air not being installed in zsh path on macOS air-verse/air#558
- 5: https://github.com/air-verse/air/blob/master/install.sh
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Looking for CONTRIBUTING or setup docs ==="
fd -a -i 'contrib|setup|install' -type f | head -20
echo
echo "=== Check .air.toml content ==="
cat napkin-backend/.air.toml
echo
echo "=== Check if there's a Makefile or setup script ==="
find . -maxdepth 2 -name "Makefile" -o -name "*.sh" -o -name "setup*" 2>/dev/null | head -10Repository: acmutsa/napkin
Length of output: 294
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Looking for docs ==="
find . -maxdepth 2 -iname "*readme*" -o -iname "*contrib*" -o -iname "*setup*" 2>/dev/null
echo
echo "=== Full README.md content ==="
if [ -f README.md ]; then
cat README.md
fi
echo
echo "=== Check .air.toml content ==="
cat napkin-backend/.air.toml
echo
echo "=== Check package.json scripts section ==="
jq '.scripts' package.jsonRepository: acmutsa/napkin
Length of output: 3049
Simplify to PATH-based air command.
The hardcoded $(go env GOPATH)/bin/air uses POSIX command substitution which breaks in non-POSIX shells (cmd.exe, PowerShell). Air installs to $(go env GOPATH)/bin by default via go install, but the standard practice is to add it to PATH and invoke air directly—matching Go ecosystem conventions. The .air.toml config is already present, so air will work once PATH is configured per Air's setup documentation.
Suggested change
- "dev:backend": "cd napkin-backend && \"$(go env GOPATH)/bin/air\"",
+ "dev:backend": "cd napkin-backend && air",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "dev:backend": "cd napkin-backend && \"$(go env GOPATH)/bin/air\"", | |
| "dev:backend": "cd napkin-backend && air", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 9, The dev:backend npm script currently invokes air via
a hardcoded POSIX command substitution ("$(go env GOPATH)/bin/air") which breaks
on non-POSIX shells; update the dev:backend script to call the air executable
directly (use "air") so it relies on PATH and works across shells, ensuring
users have installed air and added its install directory to PATH per Air's setup
instructions.
Fly.io Launch config files
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9120824. Configure here.
| setNodes((nds) => applyNodeChanges(changes, nds)); | ||
| setNodeInheritance({}); | ||
| }, | ||
| [], |
There was a problem hiding this comment.
Inheritance badges clear on every node interaction
Medium Severity
setNodeInheritance({}) is called inside onNodesChange and onEdgesChange, which fire on every change type — including position (drag), selection (click), and dimensions. This means compiler-inherited badges vanish the moment a user clicks or drags any node, not just on structural edits. The intent per the comment is to clear "when the user keeps editing," but ReactFlow change events include non-editing interactions.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9120824. Configure here.
| memory = '256mb' | ||
| cpu_kind = 'shared' | ||
| cpus = 1 | ||
| memory_mb = 256 |
There was a problem hiding this comment.
Duplicate memory configuration in fly.toml VM block
Medium Severity
The [[vm]] section specifies both memory = '256mb' and memory_mb = 256. These are redundant Fly.io VM memory settings. Having both risks ambiguity — if one is changed without the other, the effective value becomes unpredictable depending on which field Fly prioritizes.
Reviewed by Cursor Bugbot for commit 9120824. Configure here.
1 participant
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Note
Medium Risk
Medium risk because it changes the canvas-to-compiler contract (node kind/ports/edges + new IR fields) and compilation wiring logic, which could break existing graphs or Terraform output if the registries diverge.
Overview
Canvas nodes/edges are now kind- and port-aware end-to-end. The frontend introduces a
kindsregistry (ports, colors, terraform types) used to populate the toolbox, create nodes, and reject invalid edge connections (mismatched/unknown ports) with an inline error banner.Compile/analyze calls were hardened and made configurable. Frontend requests now use
apiBase()(env-driven) and send the fullintentGraphpayload toPOST /api/compile; compile results open in a new dialog with copy-to-clipboard, and the backend can return aninheritedmap which the UI surfaces as per-node badges.Backend Terraform compilation now consumes explicit ported edges and tracks inferred bindings. The compiler IR gains region + directed edges, adds an edge wiring registry (
tf_edges.go) that fails on unsupported connections, and emits inheritance metadata alongside HCL; analysis structs also get JSON tags. Misc: Fly config updated, shadcn/tailwind CSS moved toindex.css,package-lock.jsonremoved, and dev files/ignores adjusted.Reviewed by Cursor Bugbot for commit 9120824. Bugbot is set up for automated code reviews on this repo. Configure here.