Skip to content

Staging->main#391

Merged
galshubeli merged 42 commits intomainfrom
staging
Feb 16, 2026
Merged

Staging->main#391
galshubeli merged 42 commits intomainfrom
staging

Conversation

@galshubeli
Copy link
Copy Markdown
Collaborator

@galshubeli galshubeli commented Feb 16, 2026

Summary by CodeRabbit

  • New Features

    • Per-user memory TTL configuration for automatic memory expiration.
    • HSTS header enabled for stronger transport security.
  • Documentation

    • Memory TTL usage and example added to docs and env sample.
  • Refactor

    • Graph visualization migrated to a canvas-based rendering approach.
    • Frontend dependencies updated (UI graph library swapped).
  • Chores

    • Docker build improvements and dependency updates; added automated update for CI tooling.
  • Tests

    • Added tests verifying HSTS header presence and content.

Anchel123 and others added 30 commits December 30, 2025 13:53
…react-force-graph-2d

- Added @falkordb/canvas dependency to package.json and package-lock.json.
- Refactored SchemaViewer component to utilize FalkorDBCanvas for rendering schema data.
- Updated schema data handling to remap node IDs and create a nodesMap for efficient access.
- Removed all references and dependencies related to react-force-graph-2d.
- Implemented dynamic loading of canvas and adjusted theme colors for better visualization.
- Enhanced zoom and center functionalities to work with the new canvas implementation.
Bumps the pip group with 1 update in the / directory: [urllib3](https://github.com/urllib3/urllib3).


Updates `urllib3` from 2.6.2 to 2.6.3
- [Release notes](https://github.com/urllib3/urllib3/releases)
- [Changelog](https://github.com/urllib3/urllib3/blob/main/CHANGES.rst)
- [Commits](urllib3/urllib3@2.6.2...2.6.3)

---
updated-dependencies:
- dependency-name: urllib3
  dependency-version: 2.6.3
  dependency-type: indirect
  dependency-group: pip
...

Signed-off-by: dependabot[bot] <support@github.com>
Bump urllib3 from 2.6.2 to 2.6.3 in the pip group across 1 directory
Co-authored-by: gkorland <753206+gkorland@users.noreply.github.com>
Co-authored-by: gkorland <753206+gkorland@users.noreply.github.com>
Add HSTS header to prevent MITM attacks
Bumps [jsonschema](https://github.com/python-jsonschema/jsonschema) from 4.25.1 to 4.26.0.
- [Release notes](https://github.com/python-jsonschema/jsonschema/releases)
- [Changelog](https://github.com/python-jsonschema/jsonschema/blob/main/CHANGELOG.rst)
- [Commits](python-jsonschema/jsonschema@v4.25.1...v4.26.0)

---
updated-dependencies:
- dependency-name: jsonschema
  dependency-version: 4.26.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
feat: integrate @falkordb/canvas for schema visualization and remove …
@galshubeli galshubeli requested a review from Copilot February 16, 2026 07:39
@overcut-ai
Copy link
Copy Markdown

overcut-ai bot commented Feb 16, 2026

Completed Working on "Code Review"

✅ Workflow completed successfully.


👉 View complete log

@railway-app railway-app bot temporarily deployed to queryweaver / QueryWeaver-pr-391 February 16, 2026 07:39 Destroyed
@railway-app
Copy link
Copy Markdown

railway-app bot commented Feb 16, 2026

🚅 Deployed to the QueryWeaver-pr-391 environment in queryweaver

Service Status Web Updated (UTC)
QueryWeaver ✅ Success (View Logs) Web Feb 16, 2026 at 9:44 am

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 16, 2026

📝 Walkthrough

Walkthrough

Introduces optional per-user Memory TTL (env/config + Redis EXPIRE), adds HSTS header to API responses, migrates frontend graph from ForceGraph2D to FalkorDBCanvas (with Preact), updates dependencies and Docker/node installation, and adds tests for the HSTS header and minor repo metadata changes.

Changes

Cohort / File(s) Summary
Documentation & Examples
​.env.example, README.md, .github/wordlist.txt
Added Memory TTL documentation and examples (MEMORY_TTL_SECONDS) and appended "TTL" to the repository wordlist.
Dependencies & Frontend
Pipfile, app/package.json, app/src/components/schema/SchemaViewer.tsx
Bumped jsonschema; removed react-force-graph-2d, added @falkordb/canvas and preact; complete refactor of SchemaViewer to use FalkorDBCanvas, changed SchemaNode/SchemaLink IDs to numeric and added nodesMap and userId.
Infrastructure & Docker
Dockerfile, .github/dependabot.yml
Added apt packages (curl, ca-certificates, gnupg) and reworked Node.js install via NodeSource; added Dependabot config for GitHub Actions updates.
API Security & Memory Backend
api/app_factory.py, api/memory/graphiti_tool.py
Added Strict-Transport-Security header middleware; introduced MEMORY_TTL_SECONDS env var, per-instance memory_db_name, RedisError handling, and an internal _refresh_ttl() call to set Redis EXPIRE for memory graphs.
Tests
tests/test_hsts_header.py
New tests asserting presence and directives of the Strict-Transport-Security header on root and /graphs endpoints.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Client
    participant API as FastAPI Middleware
    participant MemoryTool
    participant Redis as Redis Server

    Client->>API: request (creates/uses memory graph)
    API->>MemoryTool: invoke create()/update graph
    MemoryTool->>Redis: ensure index / persist graph
    MemoryTool->>Redis: EXPIRE <memory_key> (MEMORY_TTL_SECONDS)
    Redis-->>MemoryTool: OK
    MemoryTool-->>API: return result
    API-->>Client: response (includes Strict-Transport-Security header)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Poem

🐰 I stitched a clock for memory's keep,
Set Redis naps and secrets deep,
A header guards the rolling tide,
Canvas paints where nodes abide—
Hop, hop, these changes take a leap!

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Staging->main' is vague and non-descriptive, failing to convey meaningful information about the changeset's primary purpose or content. Use a descriptive title that summarizes the main change, such as 'Add Memory TTL, HSTS headers, and FalkorDB Canvas integration' or focus on the most critical change.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch staging

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 16, 2026

Dependency Review

The following issues were found:

  • ✅ 0 vulnerable package(s)
  • ✅ 0 package(s) with incompatible licenses
  • ✅ 0 package(s) with invalid SPDX license definitions
  • ✅ 0 package(s) with unknown licenses.
  • ⚠️ 10 packages with OpenSSF Scorecard issues.

View full job summary

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request merges changes from the staging branch to main, introducing several security enhancements, frontend improvements, and new features.

Changes:

  • Added HSTS security header to all responses to prevent man-in-the-middle attacks
  • Replaced react-force-graph-2d with @falkordb/canvas for schema visualization with improved rendering
  • Added optional TTL (time-to-live) feature for user memory graphs to enable automatic cleanup of idle data
  • Updated dependencies (jsonschema 4.26.0, urllib3 2.6.3) and improved Dockerfile Node.js installation
  • Added Dependabot configuration for GitHub Actions and improved spellcheck wordlist

Reviewed changes

Copilot reviewed 11 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/test_hsts_header.py New test file verifying HSTS header presence and configuration on all endpoints
app/src/components/schema/SchemaViewer.tsx Major refactor replacing react-force-graph-2d with @falkordb/canvas library for schema visualization
app/package.json Added @falkordb/canvas and preact dependencies, removed react-force-graph-2d
api/memory/graphiti_tool.py Added TTL support for memory graphs with optional auto-expiry via MEMORY_TTL_SECONDS env var
api/app_factory.py Added HSTS header to SecurityMiddleware for all responses
README.md Documented new MEMORY_TTL_SECONDS feature and usage
.env.example Added MEMORY_TTL_SECONDS configuration example
Pipfile & Pipfile.lock Updated jsonschema to 4.26.0 and urllib3 to 2.6.3
Dockerfile Improved Node.js installation by removing conflicting packages first
.github/dependabot.yml Added GitHub Actions ecosystem monitoring
.github/wordlist.txt Added "TTL" to spellcheck whitelist

Comment on lines +419 to +437
className="h-8 w-8 p-0 bg-card border-border text-muted-foreground hover:bg-foreground"
title="Zoom In"
>
<ZoomIn className="h-4 w-4" />
</Button>
<Button
variant="outline"
size="sm"
onClick={handleZoomOut}
className="h-8 w-8 p-0 bg-card border-border text-muted-foreground hover:bg-foreground"
title="Zoom Out"
>
<ZoomOut className="h-4 w-4" />
</Button>
<Button
variant="outline"
size="sm"
onClick={handleCenter}
className="h-8 w-8 p-0 bg-card border-border text-muted-foreground hover:bg-foreground"
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hover class changed from hover:bg-muted to hover:bg-foreground, which sets the background to the foreground text color on hover. This will likely result in poor contrast and unreadable button states. Consider using hover:bg-muted or hover:bg-accent instead for proper UI feedback.

Copilot uses AI. Check for mistakes.
Comment on lines +190 to +191
console.log("Stop");

Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug console.log statement left in production code. This should be removed as it appears to be a debugging artifact with an unclear message ("Stop").

Suggested change
console.log("Stop");

Copilot uses AI. Check for mistakes.
ctx.textAlign = 'center';
ctx.textBaseline = 'middle';
ctx.fillText(
node.displayName[1],
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessing node.displayName[1] without verifying that displayName exists on the GraphNode type. According to the convertToCanvasData function, nodes are created with a data.name property but not a displayName property. This will likely cause a runtime error. Consider using schemaNode.name instead, which is the table name from the schema data.

Suggested change
node.displayName[1],
schemaNode.name,

Copilot uses AI. Check for mistakes.
</div>
)}
{!loading && canvasLoaded && schemaData && schemaData.nodes.length > 0 && (
<falkordb-canvas ref={canvasRef} node-mode='replace' />
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a custom element <falkordb-canvas> without a TypeScript declaration will cause type errors during compilation. You need to add a TypeScript declaration file (e.g., app/src/types/falkordb-canvas.d.ts) to extend the JSX namespace with:

declare namespace JSX {
  interface IntrinsicElements {
    'falkordb-canvas': React.DetailedHTMLProps<React.HTMLAttributes<HTMLElement>, HTMLElement> & {
      ref?: React.Ref<FalkorDBCanvas>;
      'node-mode'?: string;
    };
  }
}

Copilot uses AI. Check for mistakes.
Comment on lines +92 to +93
if cls.MEMORY_TTL_SECONDS is not None:
await self._refresh_ttl()
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TTL is only refreshed during object creation, but according to the README documentation added in this PR, "The TTL is refreshed on every user interaction, so active users keep their memory." Consider calling _refresh_ttl() at the end of user interaction methods like add_new_memory, save_query_memory, retrieve_similar_queries, and search_memories to prevent active users' memory from expiring.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🤖 Fix all issues with AI agents
In @.github/dependabot.yml:
- Around line 8-11: The Dependabot entry for package-ecosystem "github-actions"
is missing target-branch and will open PRs against the default branch; update
the "github-actions" block (the package-ecosystem: "github-actions" entry) to
include target-branch: "staging" so its PRs route to the staging branch like the
pip and npm entries.

In `@api/memory/graphiti_tool.py`:
- Around line 54-58: The current top-level constant MEMORY_TTL_SECONDS will
raise ValueError during import if the environment variable is non-numeric;
change the initialization of MEMORY_TTL_SECONDS to defensively parse the env var
(use os.environ.get) and wrap int(...) in a try/except (or validate with
str.isdigit()/regex) so non-integer values result in a safe fallback (e.g.,
None) and optionally log or warn; update the symbol MEMORY_TTL_SECONDS
initialization accordingly so import-time crashes are prevented.

In `@app/package.json`:
- Line 14: Remove the unused "preact" dependency from package.json: delete the
"preact" entry so it no longer contributes to install/bundle size; keep
"@falkordb/canvas": "^0.0.29" as-is but be aware it's pre-1.0 (no code changes
to `@falkordb/canvas` required). Ensure package.json still validates (commas, JSON
structure) and run npm/yarn install and a quick project build to confirm nothing
breaks after removing "preact".

In `@app/src/components/schema/SchemaViewer.tsx`:
- Around line 189-191: Remove the debug console.log("Stop") from the
handleCenter callback in SchemaViewer (the useCallback named handleCenter) so it
no longer emits console output; simply delete the console.log line and keep the
existing logic of the callback intact.
- Around line 150-155: The current remapping in data.links uses the || fallback
which can return the original string ID and cause a type mismatch for
SchemaLink.source/target; change the mapping to explicitly resolve newSource =
oldIdToNewId.get(link.source) and newTarget = oldIdToNewId.get(link.target) and
if either is undefined either (a) throw or log and remove the link so the
missing node is surfaced, or (b) raise a TypeError with details (include
link.id, link.source, link.target) so callers know about the orphaned link;
update the code that uses data.links (the mapping block referencing oldIdToNewId
and SchemaLink.source/target) to perform this explicit check instead of using ||
fallback.
- Around line 86-90: The dynamic import in the useEffect that loads
'@falkordb/canvas' lacks error handling, so if import('@falkordb/canvas')
rejects the component silently stays in the "not loaded" state; update the
effect around import('@falkordb/canvas') (the useEffect block that calls
setCanvasLoaded) to add a .catch handler (or try/catch if using async) that logs
the error and updates component state (e.g., setCanvasLoaded(false) plus a new
setCanvasLoadError or similar boolean/string) so the UI can show an error
message or fallback UI when loading fails; ensure you continue to
setCanvasLoaded(true) on success and reference useEffect,
import('@falkordb/canvas'), setCanvasLoaded, and the new error state so
reviewers can locate the change.
- Around line 451-453: Add a side-effect import to ensure the global JSX
augmentation for the custom element is loaded: at the top of SchemaViewer.tsx
add import '@falkordb/canvas'; (no `type`) so TypeScript includes the module at
runtime and the JSX intrinsic element falkordb-canvas is recognized when
rendering the falkordb-canvas element in the SchemaViewer component.

In `@README.md`:
- Line 69: The README overstates TTL behavior — _refresh_ttl() is only invoked
from MemoryTool.create(), so update the sentence "The TTL is refreshed on every
user interaction" to accurately reflect that TTL is refreshed only when
MemoryTool.create() is called (or change the implementation to call
_refresh_ttl() on every interaction if you intend the original claim); reference
the _refresh_ttl() method and MemoryTool.create() in the updated wording so
readers know the exact trigger for TTL refresh.
🧹 Nitpick comments (3)
Dockerfile (1)

43-50: Add --no-install-recommends to the Node.js install step.

Line 48 installs nodejs without --no-install-recommends, which can pull in unnecessary packages and increase image size. This is consistent with how line 18 already uses the flag.

Proposed fix
-    && apt-get install -y nodejs \
+    && apt-get install -y --no-install-recommends nodejs \
tests/test_hsts_header.py (1)

17-43: Consider @pytest.mark.parametrize to reduce duplication.

Both test methods assert the same HSTS directives on different endpoints. A parameterized test would eliminate the repetition:

Proposed refactor
-    def test_hsts_header_present(self, client):
-        """Test that the HSTS header is present in responses."""
-        # Make a request to the root endpoint
-        response = client.get("/")
-
-        # Verify HSTS header is present
-        assert "strict-transport-security" in response.headers
-
-        # Verify header value contains required directives
-        hsts_header = response.headers["strict-transport-security"]
-        assert "max-age=31536000" in hsts_header
-        assert "includeSubDomains" in hsts_header
-        assert "preload" in hsts_header
-
-    def test_hsts_header_on_api_endpoints(self, client):
-        """Test that the HSTS header is present on API endpoints."""
-        # Test on graphs endpoint
-        response = client.get("/graphs")
-
-        # Verify HSTS header is present
-        assert "strict-transport-security" in response.headers
-
-        # Verify header value contains required directives
-        hsts_header = response.headers["strict-transport-security"]
-        assert "max-age=31536000" in hsts_header
-        assert "includeSubDomains" in hsts_header
-        assert "preload" in hsts_header
+    `@pytest.mark.parametrize`("path", ["/", "/graphs"])
+    def test_hsts_header_present(self, client, path):
+        """Test that the HSTS header is present on all endpoints."""
+        response = client.get(path)
+
+        assert "strict-transport-security" in response.headers
+        hsts_header = response.headers["strict-transport-security"]
+        assert "max-age=31536000" in hsts_header
+        assert "includeSubDomains" in hsts_header
+        assert "preload" in hsts_header
app/src/components/schema/SchemaViewer.tsx (1)

200-238: convertToCanvasData is recreated on every theme change but is also a dependency of the canvas setup effect — this triggers a full data reload on theme toggle.

Since convertToCanvasData depends on theme (via useCallback(..., [theme])), every theme change causes the function identity to change, which triggers the canvas setup useEffect (line 373), calling canvas.setData(canvasData) again. This may cause unnecessary re-layouts of the graph (losing the user's pan/zoom position). Consider separating the color/background update from the data set, or only updating colors on theme change without resetting data.

Comment thread .github/dependabot.yml
Comment on lines +8 to +11
- package-ecosystem: "github-actions"
directory: "/"
schedule:
interval: "weekly"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing target-branch: "staging" — intentional?

The pip and npm entries both target staging, but this new github-actions entry omits target-branch, so Dependabot will open PRs against the default branch (main). If the intent is to keep all dependency PRs flowing through staging first, add target-branch: "staging" here as well.

Proposed fix
   - package-ecosystem: "github-actions"
     directory: "/"
+    target-branch: "staging"
     schedule:
       interval: "weekly"
📝 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.

Suggested change
- package-ecosystem: "github-actions"
directory: "/"
schedule:
interval: "weekly"
- package-ecosystem: "github-actions"
directory: "/"
target-branch: "staging"
schedule:
interval: "weekly"
🤖 Prompt for AI Agents
In @.github/dependabot.yml around lines 8 - 11, The Dependabot entry for
package-ecosystem "github-actions" is missing target-branch and will open PRs
against the default branch; update the "github-actions" block (the
package-ecosystem: "github-actions" entry) to include target-branch: "staging"
so its PRs route to the staging branch like the pip and npm entries.

Comment on lines +54 to +58
MEMORY_TTL_SECONDS: Optional[int] = (
int(os.environ["MEMORY_TTL_SECONDS"])
if os.environ.get("MEMORY_TTL_SECONDS")
else None
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

MEMORY_TTL_SECONDS will raise ValueError at import time if the env var is set to a non-integer string.

If someone sets MEMORY_TTL_SECONDS=1w or any non-numeric value, int(os.environ["MEMORY_TTL_SECONDS"]) will crash with a ValueError during module import, taking down the entire application. Consider wrapping in a try/except or validating gracefully.

🛡️ Proposed defensive parsing
-    MEMORY_TTL_SECONDS: Optional[int] = (
-        int(os.environ["MEMORY_TTL_SECONDS"])
-        if os.environ.get("MEMORY_TTL_SECONDS")
-        else None
-    )
+    `@staticmethod`
+    def _parse_ttl() -> Optional[int]:
+        raw = os.environ.get("MEMORY_TTL_SECONDS")
+        if not raw:
+            return None
+        try:
+            return int(raw)
+        except ValueError:
+            logging.warning("Invalid MEMORY_TTL_SECONDS=%r, ignoring", raw)
+            return None
+
+    MEMORY_TTL_SECONDS: Optional[int] = _parse_ttl()
🤖 Prompt for AI Agents
In `@api/memory/graphiti_tool.py` around lines 54 - 58, The current top-level
constant MEMORY_TTL_SECONDS will raise ValueError during import if the
environment variable is non-numeric; change the initialization of
MEMORY_TTL_SECONDS to defensively parse the env var (use os.environ.get) and
wrap int(...) in a try/except (or validate with str.isdigit()/regex) so
non-integer values result in a safe fallback (e.g., None) and optionally log or
warn; update the symbol MEMORY_TTL_SECONDS initialization accordingly so
import-time crashes are prevented.

Comment thread app/package.json
"preview": "vite preview"
},
"dependencies": {
"@falkordb/canvas": "^0.0.29",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, check the current state of app/package.json to see if preact is present
cd app
echo "=== Checking app/package.json for preact and react dependencies ==="
grep -E '"(preact|react|@falkordb/canvas)"' package.json | head -20

echo -e "\n=== Full package.json snippet around line 14 ==="
sed -n '10,20p' package.json

echo -e "\n=== Full package.json snippet around line 50-60 ==="
sed -n '50,60p' package.json

Repository: FalkorDB/QueryWeaver

Length of output: 1039


🏁 Script executed:

#!/bin/bash
# Check if `@falkordb/canvas` declares preact as a peer/regular dependency
# Search for any reference to `@falkordb/canvas` in the repo
echo "=== Searching for `@falkordb/canvas` references in the codebase ==="
rg "@falkordb/canvas" --type js --type jsx --type ts --type tsx -B 2 -A 2 | head -40

echo -e "\n=== Checking if there's a lockfile that shows `@falkordb/canvas` dependencies ==="
if [ -f app/package-lock.json ]; then
  jq '.packages | to_entries[] | select(.key | contains("@falkordb/canvas")) | {key: .key, dependencies: .value.dependencies, peerDependencies: .value.peerDependencies}' app/package-lock.json 2>/dev/null | head -50
elif [ -f app/yarn.lock ]; then
  echo "yarn.lock found, checking for `@falkordb/canvas`..."
  grep -A 20 '@falkordb/canvas' app/yarn.lock | head -30
fi

echo -e "\n=== Check git diff or PR context to understand why preact was added ==="
# Look for any recent changes mentioning preact or `@falkordb/canvas`
git log -5 --oneline --all 2>/dev/null | head -10
</parameter>

Repository: FalkorDB/QueryWeaver

Length of output: 834


🏁 Script executed:

#!/bin/bash
# Search more thoroughly for preact usage in the codebase
echo "=== Searching for any preact imports or references ==="
rg "preact" --type-list | head -5
rg "preact" -t js -t json -t ts -t tsx -t jsx -i 2>/dev/null || echo "No preact references found"

echo -e "\n=== Check vite or build config ==="
if [ -f app/vite.config.ts ]; then
  echo "=== vite.config.ts content ==="
  cat app/vite.config.ts
fi

if [ -f app/vite.config.js ]; then
  echo "=== vite.config.js content ==="
  cat app/vite.config.js
fi

echo -e "\n=== Check if preact is listed in package.json but actually not needed ==="
cd app
npm ls preact 2>&1 | head -20
</parameter>

Repository: FalkorDB/QueryWeaver

Length of output: 1635


Remove the unnecessary preact dependency.

preact is not used anywhere in the codebase and is not required by @falkordb/canvas (which depends on react directly). It adds bundle bloat without purpose. Remove preact from package.json line 54.

Also note that @falkordb/canvas at 0.0.29 is a pre-1.0 release; expect potential breaking changes with future updates.

🤖 Prompt for AI Agents
In `@app/package.json` at line 14, Remove the unused "preact" dependency from
package.json: delete the "preact" entry so it no longer contributes to
install/bundle size; keep "@falkordb/canvas": "^0.0.29" as-is but be aware it's
pre-1.0 (no code changes to `@falkordb/canvas` required). Ensure package.json
still validates (commas, JSON structure) and run npm/yarn install and a quick
project build to confirm nothing breaks after removing "preact".

Comment on lines +86 to +90
useEffect(() => {
import('@falkordb/canvas').then(() => {
setCanvasLoaded(true);
});
}, []);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Dynamic import has no error handling — a failed load leaves the user with no feedback.

If import('@falkordb/canvas') rejects (e.g. network issue, corrupt chunk), canvasLoaded stays false and the canvas never renders. The user sees "No schema data available" with no indication that the canvas library failed to load.

🛡️ Proposed fix
  useEffect(() => {
-    import('@falkordb/canvas').then(() => {
-      setCanvasLoaded(true);
-    });
+    import('@falkordb/canvas')
+      .then(() => setCanvasLoaded(true))
+      .catch((err) => {
+        console.error('Failed to load `@falkordb/canvas`', err);
+        toast({
+          title: 'Schema Viewer Error',
+          description: 'Failed to load graph visualization library',
+          variant: 'destructive',
+        });
+      });
  }, []);
🤖 Prompt for AI Agents
In `@app/src/components/schema/SchemaViewer.tsx` around lines 86 - 90, The dynamic
import in the useEffect that loads '@falkordb/canvas' lacks error handling, so
if import('@falkordb/canvas') rejects the component silently stays in the "not
loaded" state; update the effect around import('@falkordb/canvas') (the
useEffect block that calls setCanvasLoaded) to add a .catch handler (or
try/catch if using async) that logs the error and updates component state (e.g.,
setCanvasLoaded(false) plus a new setCanvasLoadError or similar boolean/string)
so the UI can show an error message or fallback UI when loading fails; ensure
you continue to setCanvasLoaded(true) on success and reference useEffect,
import('@falkordb/canvas'), setCanvasLoaded, and the new error state so
reviewers can locate the change.

Comment on lines +150 to +155
// Update links to use the new node IDs
data.links = data.links.map((link) => ({
...link,
source: oldIdToNewId.get(link.source) || link.source,
target: oldIdToNewId.get(link.target) || link.target,
}));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Link remapping fallback silently preserves the original string ID, causing a type mismatch.

When oldIdToNewId.get(link.source) returns undefined (i.e., a link references a node not present in the nodes array), the || fallback yields link.source which is a string — but SchemaLink.source is typed as number. This produces a silent type mismatch that will result in broken/orphaned links in the canvas.

Use a stricter approach that surfaces missing nodes:

🐛 Proposed fix
      // Update links to use the new node IDs
-      data.links = data.links.map((link) => ({
-        ...link,
-        source: oldIdToNewId.get(link.source) || link.source,
-        target: oldIdToNewId.get(link.target) || link.target,
-      }));
+      data.links = data.links.reduce<typeof data.links>((acc, link) => {
+        const source = oldIdToNewId.get(link.source);
+        const target = oldIdToNewId.get(link.target);
+        if (source !== undefined && target !== undefined) {
+          acc.push({ ...link, source, target });
+        } else {
+          console.warn('Dropping link with unknown node ID', link);
+        }
+        return acc;
+      }, []);
📝 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.

Suggested change
// Update links to use the new node IDs
data.links = data.links.map((link) => ({
...link,
source: oldIdToNewId.get(link.source) || link.source,
target: oldIdToNewId.get(link.target) || link.target,
}));
// Update links to use the new node IDs
data.links = data.links.reduce<typeof data.links>((acc, link) => {
const source = oldIdToNewId.get(link.source);
const target = oldIdToNewId.get(link.target);
if (source !== undefined && target !== undefined) {
acc.push({ ...link, source, target });
} else {
console.warn('Dropping link with unknown node ID', link);
}
return acc;
}, []);
🤖 Prompt for AI Agents
In `@app/src/components/schema/SchemaViewer.tsx` around lines 150 - 155, The
current remapping in data.links uses the || fallback which can return the
original string ID and cause a type mismatch for SchemaLink.source/target;
change the mapping to explicitly resolve newSource =
oldIdToNewId.get(link.source) and newTarget = oldIdToNewId.get(link.target) and
if either is undefined either (a) throw or log and remove the link so the
missing node is surfaced, or (b) raise a TypeError with details (include
link.id, link.source, link.target) so callers know about the orphaned link;
update the code that uses data.links (the mapping block referencing oldIdToNewId
and SchemaLink.source/target) to perform this explicit check instead of using ||
fallback.

Comment on lines +189 to +191
const handleCenter = useCallback(() => {
console.log("Stop");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove debug console.log("Stop").

This is a leftover debug statement that shouldn't ship to production.

🧹 Proposed fix
  const handleCenter = useCallback(() => {
-    console.log("Stop");
-
     const canvas = canvasRef.current;
📝 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.

Suggested change
const handleCenter = useCallback(() => {
console.log("Stop");
const handleCenter = useCallback(() => {
const canvas = canvasRef.current;
🤖 Prompt for AI Agents
In `@app/src/components/schema/SchemaViewer.tsx` around lines 189 - 191, Remove
the debug console.log("Stop") from the handleCenter callback in SchemaViewer
(the useCallback named handleCenter) so it no longer emits console output;
simply delete the console.log line and keep the existing logic of the callback
intact.

Comment on lines +451 to +453
{!loading && canvasLoaded && schemaData && schemaData.nodes.length > 0 && (
<falkordb-canvas ref={canvasRef} node-mode='replace' />
)}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Check for falkordb-canvas type declarations and JSX IntrinsicElements
rg -i "falkordb-canvas" --type ts --type tsx --type d.ts -C5

Repository: FalkorDB/QueryWeaver

Length of output: 91


🏁 Script executed:

# Search for JSX.IntrinsicElements declarations in the codebase
rg "JSX\s*{\s*namespace|IntrinsicElements" --type ts --type tsx --type d.ts -C3

Repository: FalkorDB/QueryWeaver

Length of output: 190


🏁 Script executed:

# Check for any global type definitions or d.ts files
find . -name "*.d.ts" -o -name "global.d.ts" -o -name "types.d.ts" | grep -v node_modules | head -20

Repository: FalkorDB/QueryWeaver

Length of output: 46


🏁 Script executed:

# Check package.json for `@falkordb/canvas` dependency
cat package.json | grep -A5 -B5 "falkordb" || echo "Not found in package.json"

Repository: FalkorDB/QueryWeaver

Length of output: 89


🏁 Script executed:

# Look for `@falkordb/canvas` in node_modules and check its type definitions
find . -path "*/node_modules/@falkordb/canvas*" -name "*.d.ts" -o -path "*/node_modules/@falkordb/canvas*" -name "package.json" 2>/dev/null | head -10

Repository: FalkorDB/QueryWeaver

Length of output: 46


🏁 Script executed:

# Search for falkordb-canvas using correct file type
rg "falkordb-canvas" --type ts -C5

Repository: FalkorDB/QueryWeaver

Length of output: 1856


🏁 Script executed:

# Search for JSX IntrinsicElements declarations
rg "IntrinsicElements" --type ts -C3

Repository: FalkorDB/QueryWeaver

Length of output: 46


🏁 Script executed:

# Look for global type definitions
find . -name "*.d.ts" -not -path "*/node_modules/*" 2>/dev/null

Repository: FalkorDB/QueryWeaver

Length of output: 46


🏁 Script executed:

# Check the actual SchemaViewer.tsx file to see how it's importing/using falkordb-canvas
find . -name "SchemaViewer.tsx" -exec cat {} \; 2>/dev/null

Repository: FalkorDB/QueryWeaver

Length of output: 14812


🏁 Script executed:

# Search for `@falkordb` in package.json and related config files
find . -maxdepth 2 -name "package.json" -o -name "package-lock.json" | xargs grep -l "falkordb" 2>/dev/null || echo "No falkordb found"

Repository: FalkorDB/QueryWeaver

Length of output: 126


🏁 Script executed:

# Check if there's a TypeScript errors in the project
ls -la tsconfig.json 2>/dev/null && cat tsconfig.json || echo "No tsconfig.json found"

Repository: FalkorDB/QueryWeaver

Length of output: 86


🏁 Script executed:

# Check app/package.json for `@falkordb/canvas`
cat ./app/package.json | grep -A10 -B10 "falkordb\|@falkordb"

Repository: FalkorDB/QueryWeaver

Length of output: 771


🏁 Script executed:

# Check if `@falkordb/canvas` is installed in node_modules
find ./app/node_modules -name "@falkordb" -type d 2>/dev/null || echo "Not found"

Repository: FalkorDB/QueryWeaver

Length of output: 73


🏁 Script executed:

# Search for any JSX type augmentation or global declarations
rg "declare.*JSX\|namespace.*JSX\|IntrinsicElements" . --type ts -B2 -A5 2>/dev/null || echo "Not found"

Repository: FalkorDB/QueryWeaver

Length of output: 73


🏁 Script executed:

# Check if the custom element might be defined elsewhere
rg "falkordb-canvas" . --type ts --type d.ts -B10 -A10 2>/dev/null | head -100

Repository: FalkorDB/QueryWeaver

Length of output: 46


🏁 Script executed:

# Look for tsconfig in the app directory
find ./app -name "tsconfig.json" -o -name "tsconfig*.json" | head -5

Repository: FalkorDB/QueryWeaver

Length of output: 159


🏁 Script executed:

# Check if there are any .d.ts files that might declare the element
find ./app -name "*.d.ts" -not -path "*/node_modules/*" 2>/dev/null

Repository: FalkorDB/QueryWeaver

Length of output: 46


🏁 Script executed:

# Check the main tsconfig.json in app
cat ./app/tsconfig.json

Repository: FalkorDB/QueryWeaver

Length of output: 391


🏁 Script executed:

# Check if there's a vite-env.d.ts or similar
find ./app/src -name "*.d.ts" 2>/dev/null

Repository: FalkorDB/QueryWeaver

Length of output: 46


🏁 Script executed:

# Look for any env or global declaration files
find ./app -maxdepth 3 -name "*env.d.ts" -o -name "vite-env.d.ts" -o -name "global.d.ts"

Repository: FalkorDB/QueryWeaver

Length of output: 46


🏁 Script executed:

# Search for any declarations that might handle falkordb-canvas
rg "falkordb" ./app --type ts -B5 -A5 2>/dev/null | head -150

Repository: FalkorDB/QueryWeaver

Length of output: 5559


🏁 Script executed:

# Check what's actually exported from `@falkordb/canvas`
find ./app/node_modules/@falkordb -name "*.d.ts" 2>/dev/null | head -20

Repository: FalkorDB/QueryWeaver

Length of output: 46


🏁 Script executed:

# Try to locate `@falkordb/canvas` package.json to check typings entry
find . -path "*/@falkordb/canvas*" -name "package.json" 2>/dev/null

Repository: FalkorDB/QueryWeaver

Length of output: 46


🏁 Script executed:

# Check the actual TypeScript compilation - look for any error logs or build outputs
find ./app -name "dist" -o -name "build" -o -name ".vite" | head -5

Repository: FalkorDB/QueryWeaver

Length of output: 46


🏁 Script executed:

# Try a different approach - check if there are any vite or build configuration files
cat ./app/vite.config.ts 2>/dev/null || cat ./app/vite.config.js 2>/dev/null || echo "No vite config found"

Repository: FalkorDB/QueryWeaver

Length of output: 837


🏁 Script executed:

# Search for any React type definitions that might augment JSX
find ./app -name "react.d.ts" -o -name "index.d.ts" | xargs grep -l "IntrinsicElements\|falkordb" 2>/dev/null || echo "None found"

Repository: FalkorDB/QueryWeaver

Length of output: 74


🏁 Script executed:

# Check if `@falkordb/canvas` might declare elements via a global augmentation
rg "declare\s+global|module\s+[\'\"]@falkordb" ./app --type ts --type d.ts 2>/dev/null || echo "Not found"

Repository: FalkorDB/QueryWeaver

Length of output: 73


🌐 Web query:

@falkordb/canvas npm package type definitions JSX IntrinsicElements

💡 Result:

@falkordb/canvas already ships TypeScript types and it declares the custom element for JSX.

  • The package points TypeScript at dist/index.d.ts via "types": "dist/index.d.ts" (and exports["."].types). [1]
  • In dist/index.d.ts it does a declare global { namespace JSX { interface IntrinsicElements { ... }}} augmentation that adds the "falkordb-canvas" intrinsic element, plus a HTMLElementTagNameMap entry. [2]
  • The props it exposes on the JSX tag include:
    • 'node-mode'?: CanvasRenderMode
    • 'link-mode'?: CanvasRenderMode [2]
  • CanvasRenderMode is defined as 'before' | 'after' | 'replace'. [3]
  • TypeScript’s JSX intrinsic element checking works exactly via JSX.IntrinsicElements augmentation. [4]

If TS/JSX still says "Property 'falkordb-canvas' does not exist on type JSX.IntrinsicElements'":

Make sure the package’s global augmentation is included by importing it somewhere that’s in your TS program, e.g. in your app entry or a global.d.ts:

import "@falkordb/canvas";

This forces dist/index.d.ts (and its declare global { JSX.IntrinsicElements ... }) to be loaded. [2][4]

Sources: [1] package.json types/exports; [2] dist/index.d.ts global JSX + tag map; [3] dist/canvas-types.d.ts CanvasRenderMode; [4] TypeScript JSX docs on JSX.IntrinsicElements.


🏁 Script executed:

# Check if there's a compiled output or build artifacts
ls -la ./app/dist 2>/dev/null || echo "No dist directory"

Repository: FalkorDB/QueryWeaver

Length of output: 81


🏁 Script executed:

# Check app/src for any setup or entry files that might have the import
cat ./app/src/main.tsx 2>/dev/null || cat ./app/src/index.tsx 2>/dev/null || echo "Not found"

Repository: FalkorDB/QueryWeaver

Length of output: 896


🏁 Script executed:

# Check if there are any type declaration files in src that might augment JSX globally
find ./app/src -name "*.d.ts" -exec cat {} \;

Repository: FalkorDB/QueryWeaver

Length of output: 46


Add a side-effect import of @falkordb/canvas to load JSX type declarations.

The code uses a type-only import (import type { ... } from '@falkordb/canvas'), which TypeScript erases at compile time and does not execute global side effects. The @falkordb/canvas package declares the falkordb-canvas JSX element via a global augmentation in its type definitions, but this augmentation is never loaded. This will produce a TypeScript compile error: Property 'falkordb-canvas' does not exist on type JSX.IntrinsicElements.

Add import '@falkordb/canvas'; (without the type keyword) at the top of the file to ensure the global JSX augmentation is loaded before the component uses the element.

🤖 Prompt for AI Agents
In `@app/src/components/schema/SchemaViewer.tsx` around lines 451 - 453, Add a
side-effect import to ensure the global JSX augmentation for the custom element
is loaded: at the top of SchemaViewer.tsx add import '@falkordb/canvas'; (no
`type`) so TypeScript includes the module at runtime and the JSX intrinsic
element falkordb-canvas is recognized when rendering the falkordb-canvas element
in the SchemaViewer component.

Comment thread README.md
MEMORY_TTL_SECONDS=604800
```

The TTL is refreshed on every user interaction, so active users keep their memory.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Documentation may overstate the TTL refresh guarantee.

Line 69 says "The TTL is refreshed on every user interaction." As noted in the graphiti_tool.py review, _refresh_ttl() is only called inside MemoryTool.create(). If create() isn't invoked on every interaction, this statement is misleading. Please verify the call path and update the wording if needed.

🤖 Prompt for AI Agents
In `@README.md` at line 69, The README overstates TTL behavior — _refresh_ttl() is
only invoked from MemoryTool.create(), so update the sentence "The TTL is
refreshed on every user interaction" to accurately reflect that TTL is refreshed
only when MemoryTool.create() is called (or change the implementation to call
_refresh_ttl() on every interaction if you intend the original claim); reference
the _refresh_ttl() method and MemoryTool.create() in the updated wording so
readers know the exact trigger for TTL refresh.

@galshubeli galshubeli requested a review from gkorland February 16, 2026 07:56
Copy link
Copy Markdown

@overcut-ai overcut-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

  • Importance counts: BLOCKER 1 · CRITICAL 0 · MAJOR 6 · MINOR 0 · SUGGESTION 0 · PRAISE 0
  • Key themes:
    1. Memory retention feature does not actually refresh or enforce TTL reliably, and failures currently degrade silently.
    2. Schema viewer refactor regresses basic rendering (missing labels) and link integrity after node ID remapping.
    3. Security middleware ordering and the accompanying HSTS tests need tightening—the header is not guaranteed to be applied, and the tests spin up the full app over HTTP with real DB connections.

Next steps

  1. Reorder the FastAPI middleware stack so SecurityMiddleware wraps all subsequent middleware, and adjust the HSTS tests to avoid real FalkorDB dependencies while validating HTTPS behavior.
  2. Ensure every memory read/write path calls _refresh_ttl and surfaces TTL refresh failures instead of silently continuing when Redis/FalkorDB is unavailable.
  3. Fix the schema canvas data mapping so node titles use real schema names and drop/log edges whose endpoints are missing after the ID remap.

Blocking issues remain (middleware ordering), so the review is left in COMMENT state pending fixes.

async def _refresh_ttl(self) -> None:
"""Set a TTL on the memory graph key using Redis EXPIRE."""
try:
await db.execute_command("EXPIRE", self.memory_db_name, self.MEMORY_TTL_SECONDS)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR]: _refresh_ttl only logs RedisError and then returns control to the caller, so a FalkorDB/Redis outage silently disables TTL enforcement—graphs never expire but nothing fails or alerts. That means the “memory auto-expiry” feature can be broken for weeks without anyone knowing. Please fail MemoryTool.create (or otherwise signal the failure) when the EXPIRE call raises so the request can retry/alert rather than silently degrading security/privacy guarantees.

await driver.execute_query(f"CREATE VECTOR INDEX FOR (p:Query) ON (p.embeddings) OPTIONS {{dimension:{vector_size}, similarityFunction:'euclidean'}}")

if cls.MEMORY_TTL_SECONDS is not None:
await self._refresh_ttl()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR]: The README now promises “The TTL is refreshed on every user interaction,” but _refresh_ttl() is only invoked once inside MemoryTool.create. After MEMORY_TTL_SECONDS elapses the user graph expires even if the user was actively chatting, which defeats the idle-based TTL requirement. Please refresh the TTL on every read/write path (e.g. search, add_new_memory, save_query_memory) so each interaction extends the expiry.

ctx.textAlign = 'center';
ctx.textBaseline = 'middle';
ctx.fillText(
node.displayName[1],
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR]: Custom node rendering now uses node.displayName[1] for the header text, but the canvas data never sets a displayName array—nodes only have data.name. As a result every table renders without a label. Please render schemaNode.name (or node.data?.name) instead so titles show up again.

// Update links to use the new node IDs
data.links = data.links.map((link) => ({
...link,
source: oldIdToNewId.get(link.source) || link.source,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR]: When remapping nodes to numeric IDs, links fall back to the original string ID if oldIdToNewId has no match: oldIdToNewId.get(link.source) || link.source. That produces mixed string/number IDs in the same graph, so any link whose endpoint wasn�t mapped now references a non-existent node in the canvas and silently disappears. Instead, fail/skip/log these edges so the rendered graph stays consistent.

Comment thread api/app_factory.py
# max-age=31536000: 1 year in seconds
# includeSubDomains: apply to all subdomains
# preload: eligible for browser HSTS preload lists
hsts_value = "max-age=31536000; includeSubDomains; preload"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[BLOCKER]: Middleware is stacked LIFO, so adding SecurityMiddleware inside RequestValidationMiddleware.dispatch means it executes after ProxyHeadersMiddleware/SessionMiddleware have already built their response. Redirects, cookies, etc. leave the app before the HSTS header is injected, defeating the goal of globally enforcing HSTS. Please register the HSTS middleware at FastAPI app creation (before ProxyHeaders/Session) so every response carries the header.

Comment thread tests/test_hsts_header.py
"""
import pytest
from fastapi.testclient import TestClient
from api.index import app
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR]: Importing app from api.index spins up the full production app (including FalkorDB/Redis connections) at import time. These tests will fail in CI or local dev whenever no database is running, even though the tests only need to inspect headers. Please instantiate a lightweight app for the test (e.g., call the factory with dependencies patched or provide a settings flag to skip DB initialization) so the HSTS tests don’t require a live FalkorDB instance.

Comment thread tests/test_hsts_header.py
"""Create a test client."""
return TestClient(app)

def test_hsts_header_present(self, client):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR]: These tests call TestClient(app) over HTTP and merely assert that the header exists. HSTS is ignored on insecure transport, so this test proves only that FastAPI can set a header—not that the middleware behaves correctly under HTTPS. Please adjust the client to use an HTTPS base URL or patch the middleware to emit the header only when request.url.scheme == 'https', and cover both the HTTPS and HTTP cases to ensure the production configuration is actually exercised.

@railway-app railway-app bot temporarily deployed to queryweaver / QueryWeaver-pr-391 February 16, 2026 09:32 Destroyed
@railway-app railway-app bot temporarily deployed to queryweaver / staging February 16, 2026 09:32 Inactive
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@Dockerfile`:
- Around line 43-50: Scope the "|| true" to only the removal step and add
--no-install-recommends to the install: change the RUN chain so apt-get update
is not short-circuited by the "|| true" (i.e., use apt-get update && (apt-get
remove -y nodejs || true) && rm -rf /var/lib/apt/lists/* && curl -fsSL
https://deb.nodesource.com/setup_22.x | bash - && apt-get update && apt-get
install -y --no-install-recommends nodejs && rm -rf /var/lib/apt/lists/* && node
--version && npm --version), ensuring the "remove" fallback only applies to the
remove command and the install uses --no-install-recommends.
- Around line 69-70: The Dockerfile contains a redundant COPY . . (the later
COPY . . duplicates the earlier copy of the app/ directory); remove the second
COPY . . (or consolidate into a single targeted COPY that copies only the
necessary source files) and add a .dockerignore to exclude unnecessary context
(e.g., .git, node_modules, .venv, .env, app/dist) so builds are smaller and
faster; locate the duplicate COPY . . entry and either delete it or replace both
COPY operations with a single explicit COPY that targets the application source,
then add a .dockerignore file listing the unwanted files/directories.

Comment thread Dockerfile
Comment on lines +43 to +50
RUN apt-get update \
&& apt-get remove -y nodejs || true \
&& rm -rf /var/lib/apt/lists/* \
&& curl -fsSL https://deb.nodesource.com/setup_22.x | bash - \
&& apt-get update \
&& apt-get install -y nodejs \
&& rm -rf /var/lib/apt/lists/* \
&& node --version && npm --version
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Shell operator precedence issue silently swallows apt-get update failure; also add --no-install-recommends.

Because && and || have equal precedence and are left-associative in shell, the expression apt-get update && apt-get remove -y nodejs || true && ... evaluates as ((apt-get update && apt-get remove ...) || true) && .... If apt-get update on line 43 fails, the || true catches it and the rest of the chain proceeds silently with a stale/missing package index. Scope the || true to only the remove command.

Additionally, apt-get install -y nodejs on line 48 is missing --no-install-recommends, which would pull in unnecessary recommended packages and increase image size (flagged by Trivy DS-0029).

Proposed fix
-RUN apt-get update \
-    && apt-get remove -y nodejs || true \
-    && rm -rf /var/lib/apt/lists/* \
-    && curl -fsSL https://deb.nodesource.com/setup_22.x | bash - \
-    && apt-get update \
-    && apt-get install -y nodejs \
-    && rm -rf /var/lib/apt/lists/* \
-    && node --version && npm --version
+RUN apt-get update \
+    && (apt-get remove -y nodejs || true) \
+    && rm -rf /var/lib/apt/lists/* \
+    && curl -fsSL https://deb.nodesource.com/setup_22.x | bash - \
+    && apt-get update \
+    && apt-get install -y --no-install-recommends nodejs \
+    && rm -rf /var/lib/apt/lists/* \
+    && node --version && npm --version
🧰 Tools
🪛 Trivy (0.69.1)

[error] 43-50: 'apt-get' missing '--no-install-recommends'

'--no-install-recommends' flag is missed: 'apt-get update && apt-get remove -y nodejs || true && rm -rf /var/lib/apt/lists/* && curl -fsSL https://deb.nodesource.com/setup_22.x | bash - && apt-get update && apt-get install -y nodejs && rm -rf /var/lib/apt/lists/* && node --version && npm --version'

Rule: DS-0029

Learn more

(IaC/Dockerfile)

🤖 Prompt for AI Agents
In `@Dockerfile` around lines 43 - 50, Scope the "|| true" to only the removal
step and add --no-install-recommends to the install: change the RUN chain so
apt-get update is not short-circuited by the "|| true" (i.e., use apt-get update
&& (apt-get remove -y nodejs || true) && rm -rf /var/lib/apt/lists/* && curl
-fsSL https://deb.nodesource.com/setup_22.x | bash - && apt-get update &&
apt-get install -y --no-install-recommends nodejs && rm -rf /var/lib/apt/lists/*
&& node --version && npm --version), ensuring the "remove" fallback only applies
to the remove command and the install uses --no-install-recommends.

Comment thread Dockerfile
Comment on lines +69 to 70
# Copy application code
COPY . .
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if .dockerignore exists and what it excludes related to app/ build artifacts
if [ -f .dockerignore ]; then
  echo "=== .dockerignore contents ==="
  cat .dockerignore
else
  echo "WARNING: No .dockerignore found"
fi

Repository: FalkorDB/QueryWeaver

Length of output: 95


🏁 Script executed:

#!/bin/bash
# Check the Dockerfile around lines 69-70 for full context
echo "=== Dockerfile (full content) ==="
if [ -f Dockerfile ]; then
  cat -n Dockerfile
else
  echo "Dockerfile not found"
fi

echo ""
echo "=== .gitignore contents ==="
if [ -f .gitignore ]; then
  cat .gitignore
else
  echo "No .gitignore found"
fi

echo ""
echo "=== Check if app/node_modules or app/dist exist in repo ==="
git ls-files | grep -E '(app/node_modules|app/dist|app/build)' | head -20 || echo "No committed build artifacts or node_modules found"

Repository: FalkorDB/QueryWeaver

Length of output: 3629


COPY . . on line 70 is redundant; the app/ directory was already copied on line 65.

The subsequent COPY . . will re-copy application code without risk of overwriting the frontend build artifacts (app/dist/), since .gitignore properly excludes both node_modules/ and /app/dist/ from the repository. However, a .dockerignore file would be beneficial to exclude unnecessary build context files (e.g., .git, .venv, .env) and improve build efficiency. Consider removing the redundant COPY . . or, if it's needed for Python application code, consolidate the two COPY operations and add a .dockerignore to exclude build directories, cache files, and version control artifacts.

🤖 Prompt for AI Agents
In `@Dockerfile` around lines 69 - 70, The Dockerfile contains a redundant COPY .
. (the later COPY . . duplicates the earlier copy of the app/ directory); remove
the second COPY . . (or consolidate into a single targeted COPY that copies only
the necessary source files) and add a .dockerignore to exclude unnecessary
context (e.g., .git, node_modules, .venv, .env, app/dist) so builds are smaller
and faster; locate the duplicate COPY . . entry and either delete it or replace
both COPY operations with a single explicit COPY that targets the application
source, then add a .dockerignore file listing the unwanted files/directories.

@galshubeli galshubeli merged commit 856fa6a into main Feb 16, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants