Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a visualizer feature: control-panel UI and route, a proxy GET endpoint, a new eReputation public endpoint to return all signed references, a seed script to populate test data, dependency and config updates, and TypeScript declaration for Next.js. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CP_UI as Control Panel UI
participant CP_Server as Control Panel Server
participant EReputation as eReputation API
participant DB as Database
User->>CP_UI: Navigate to /visualizer
CP_UI->>CP_Server: GET /api/references
CP_Server->>EReputation: GET /api/references/all (optional x-visualizer-key)
EReputation->>DB: Query signed references + author relations
DB-->>EReputation: Return reference rows
EReputation-->>CP_Server: JSON { references: [...] }
CP_Server-->>CP_UI: Proxy JSON response
CP_UI->>CP_UI: Build vis-network graph (nodes/edges) and initialize physics
User->>CP_UI: Click node
CP_UI->>CP_UI: Show detail panel (incoming/outgoing lists)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (4)
infrastructure/control-panel/package.json (1)
48-56: Consider whether@xyflow/svelteis still needed alongsidevis-network.Both
@xyflow/svelte(line 48) andvis-network(line 56) are graph visualization libraries. If no existing page consumes@xyflow/svelteafter this PR, removing it would reduce bundle size.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/control-panel/package.json` around lines 48 - 56, Check whether the package "@xyflow/svelte" is still imported/used anywhere (search for imports/require of "@xyflow/svelte" or components provided by it); if no files reference it, remove "@xyflow/svelte" from package.json dependencies, update the lockfile (run npm/yarn/pnpm install), and run the app build and tests to ensure nothing breaks; if it is used, keep it but prefer consolidating to "vis-network" where possible by refactoring imports to "vis-network" and removing "@xyflow/svelte" only after verifying feature parity.infrastructure/control-panel/src/routes/visualizer/+page.server.ts (1)
21-29: Missingresponse.okcheck before parsing JSON.If the API returns a non-2xx status with a JSON body (e.g.,
{ error: "..." }),response.json()will succeed butdata.referenceswill beundefined, silently returning an empty array with no indication of failure in the logs.Proposed fix
export const load: PageServerLoad = async ({ fetch }) => { try { const response = await fetch('/api/references'); + if (!response.ok) { + console.error('Failed to load references:', response.status, response.statusText); + return { references: [] as ReferenceEdge[] }; + } const data = await response.json(); return { references: (data.references ?? []) as ReferenceEdge[] }; } catch (error) { console.error('Error loading references for visualizer:', error); return { references: [] as ReferenceEdge[] }; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/control-panel/src/routes/visualizer/`+page.server.ts around lines 21 - 29, In load (export const load: PageServerLoad) add a check of response.ok before calling response.json(): if !response.ok, optionally await response.text() or response.json() to capture error details, log a clear error including response.status and the error body, and return { references: [] as ReferenceEdge[] }; otherwise parse JSON and return { references: (data.references ?? []) as ReferenceEdge[] }; ensure logging uses the response variable and the parsed error to avoid silently swallowing API errors.infrastructure/control-panel/src/routes/visualizer/+page.svelte (2)
226-229:in:slide/out:slidetransitions on the aside are no-ops.Svelte intro/outro transitions fire when an element enters or leaves the DOM. This
<aside>is always rendered (unconditionally); only its inner content toggles via{#ifselectedNode}. The transitions never trigger.If the intent is to animate the panel on selection change, either wrap the aside itself in an
{#ifselectedNode}block (and show the placeholder elsewhere), or move the transitions to the inner conditional blocks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/control-panel/src/routes/visualizer/`+page.svelte around lines 226 - 229, The aside element currently has in:slide/out:slide but those transitions never run because the aside is always rendered and only its inner {`#if` selectedNode} content toggles; fix by either wrapping the aside itself in the conditional (use {`#if` selectedNode} around the <aside> and render the placeholder content elsewhere) or move the in:slide and out:slide directives from the <aside> to the inner conditional blocks that actually mount/unmount (the {`#if` selectedNode} and its {:else}) so the slide transitions run when selectedNode changes.
244-303: Outgoing and incoming sections are nearly identical — consider extracting a shared snippet.The two
{#each}blocks (outgoing lines 256–300, incoming lines 317–361) share the same detail layout (type badge, score, date, content). A Svelte 5 snippet or a child component would reduce ~45 lines of duplication and make future changes (e.g., adding a field) less error-prone.Also applies to: 305-364
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/control-panel/src/routes/visualizer/`+page.svelte around lines 244 - 303, The outgoing and incoming {`#each`} blocks duplicate the same details layout (the inner <details> structure rendering item.targetLabel, item.ref.referenceType, item.ref.numericScore, item.ref.createdAt, and item.ref.content); extract that markup into a reusable Svelte child component (e.g., ReferenceDetail) or a Svelte 5 snippet that accepts the item prop and renders the badge, score, date, and content, then replace both loops to render <ReferenceDetail {item}/> (or the snippet) for selectedNode.outgoing and selectedNode.incoming to eliminate duplication and keep logic centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infrastructure/control-panel/.npmrc`:
- Line 1: Add a comment next to the engine-strict=false setting documenting why
engine enforcement was relaxed: note the package that triggered the conflict
(vis-network), the conflicting engine/version constraint, the PR or issue number
that justified the change, the date, and any planned follow-up (e.g.,
pinning/patching or reverting when upstream fixes). Place this comment
immediately above or beside the existing engine-strict=false line in .npmrc so
future contributors can see the rationale and the tracked reference.
In `@infrastructure/control-panel/src/routes/api/references/`+server.ts:
- Line 9: The fetch call that assigns response (the line "const response = await
fetch(`${baseUrl}/api/references/all`);") has no timeout and can hang; wrap the
request with an AbortController (or use AbortSignal.timeout) and pass its signal
to fetch, ensure you clear any manual timeout on success and handle the abort
error path to return/throw a controlled timeout error; update the fetch
invocation to include the signal option and add a reasonable timeout constant
near baseUrl usage.
- Around line 3-6: The server handler is reading env.VITE_EREPUTATION_BASE_URL
which uses the VITE_ prefix and will be inlined into the client bundle; change
usages to a server-only name (e.g. env.EREPUTATION_BASE_URL) in the GET
RequestHandler and the baseUrl assignment, update any references to
env.VITE_EREPUTATION_BASE_URL to env.EREPUTATION_BASE_URL (and rename the
variable in your .env / deployment secrets to EREPUTATION_BASE_URL) so the
backend URL is not exposed to the client.
In `@infrastructure/control-panel/src/routes/visualizer/`+page.svelte:
- Line 64: The current truthiness check hides a valid zero score because it uses
ref.numericScore in a boolean context; update the logic in the score formatting
to explicitly check for null/undefined (e.g., ref.numericScore != null or typeof
ref.numericScore === 'number') instead of relying on truthiness so that 0 is
rendered as "(0/5)". Apply the same change wherever similar checks occur (the
other occurrences referenced in the template around the score rendering) and
keep the constructed label (scoreLabel) behavior identical when numericScore is
present.
- Around line 95-177: The Network instance created inside onMount is never torn
down; modify the onMount callback to return a cleanup function that checks the
network variable and calls network.off(...) for any attached listeners (e.g.,
'stabilizationIterationsDone' and 'click') and then calls network.destroy();
ensure you reference the local network variable created with new
Network(container, networkData, options) and remove or disable physics
(network.setOptions) only while mounted, e.g., return () => { if (network) {
network.off('stabilizationIterationsDone'); network.off('click');
network.destroy(); } } so the Network and its DOM/event handlers are released on
component teardown.
In `@platforms/ereputation/api/src/controllers/ReferenceController.ts`:
- Around line 93-97: The author property on ref is accessed without a null check
and can be null for orphaned references; update the author object construction
in ReferenceController (the block building author: { id: ref.author.id, ename:
ref.author.ename, name: ref.author.name }) to use optional chaining and safe
fallbacks (e.g., id: ref.author?.id, ename: ref.author?.ename, name:
ref.author?.name or null) so the handler won't throw when ref.author is null,
matching the optional-chaining pattern used elsewhere in this controller.
In `@platforms/ereputation/api/src/database/seed.ts`:
- Around line 40-45: The DataSource config for SeedDataSource currently has
synchronize: true which can mutate the live DB schema; change SeedDataSource's
DataSource options to use synchronize: false, and ensure the seed script runs
any pending migrations (e.g., call the migrations runner or TypeORM's
runMigrations) before inserting seed data; if you need automatic sync only for
tests, wrap synchronize: true behind an explicit test-only check
(process.env.NODE_ENV === "test") so production/development always use
synchronize: false.
- Around line 149-154: The current use of refRepo.clear() will truncate the
entire reference table and must be replaced with a scoped delete that only
removes rows created by the seed users; change the logic that computes
existingRefCount and the deletion to count and delete only where createdBy (or
equivalent creator field) matches the known seed user IDs (e.g. the
seedUsers/knownSeedUserIds array used earlier in the file), using
refRepo.count({ where: { createdBy: In(knownSeedUserIds) } }) and
refRepo.delete({ createdBy: In(knownSeedUserIds) }) or an equivalent
queryBuilder WHERE clause instead of refRepo.clear().
In `@platforms/ereputation/api/src/index.ts`:
- Around line 88-89: The route app.get("/api/references/all") currently exposes
full PII publicly; move this route so it is registered after authMiddleware and
protect it by adding an authGuard that checks admin role (e.g., requireAdmin on
referenceController.getAllReferences), rejecting non-admins; alternatively, if
you cannot use full auth yet, implement a static API-key header check before
invoking referenceController.getAllReferences as a lightweight gate, or else
strip the sensitive content field from the response in
referenceController.getAllReferences and document that the endpoint is
intentionally public.
In `@platforms/ereputation/api/src/services/ReferenceService.ts`:
- Around line 85-91: getAllReferences() performs an unbounded query and can OOM;
change it to accept and enforce pagination or a hard cap (use the MAX_FETCH =
500 pattern used elsewhere) so it does not load all signed rows and their author
relations at once. Update the ReferenceService.getAllReferences method to take
optional limit/offset parameters (or apply a default take =
Math.min(requestLimit || DEFAULT_LIMIT, MAX_FETCH)) and use take/skip in the
repository.find call while preserving relations and order; ensure callers are
updated or provide a safe default cap if no params are passed.
---
Nitpick comments:
In `@infrastructure/control-panel/package.json`:
- Around line 48-56: Check whether the package "@xyflow/svelte" is still
imported/used anywhere (search for imports/require of "@xyflow/svelte" or
components provided by it); if no files reference it, remove "@xyflow/svelte"
from package.json dependencies, update the lockfile (run npm/yarn/pnpm install),
and run the app build and tests to ensure nothing breaks; if it is used, keep it
but prefer consolidating to "vis-network" where possible by refactoring imports
to "vis-network" and removing "@xyflow/svelte" only after verifying feature
parity.
In `@infrastructure/control-panel/src/routes/visualizer/`+page.server.ts:
- Around line 21-29: In load (export const load: PageServerLoad) add a check of
response.ok before calling response.json(): if !response.ok, optionally await
response.text() or response.json() to capture error details, log a clear error
including response.status and the error body, and return { references: [] as
ReferenceEdge[] }; otherwise parse JSON and return { references:
(data.references ?? []) as ReferenceEdge[] }; ensure logging uses the response
variable and the parsed error to avoid silently swallowing API errors.
In `@infrastructure/control-panel/src/routes/visualizer/`+page.svelte:
- Around line 226-229: The aside element currently has in:slide/out:slide but
those transitions never run because the aside is always rendered and only its
inner {`#if` selectedNode} content toggles; fix by either wrapping the aside
itself in the conditional (use {`#if` selectedNode} around the <aside> and render
the placeholder content elsewhere) or move the in:slide and out:slide directives
from the <aside> to the inner conditional blocks that actually mount/unmount
(the {`#if` selectedNode} and its {:else}) so the slide transitions run when
selectedNode changes.
- Around line 244-303: The outgoing and incoming {`#each`} blocks duplicate the
same details layout (the inner <details> structure rendering item.targetLabel,
item.ref.referenceType, item.ref.numericScore, item.ref.createdAt, and
item.ref.content); extract that markup into a reusable Svelte child component
(e.g., ReferenceDetail) or a Svelte 5 snippet that accepts the item prop and
renders the badge, score, date, and content, then replace both loops to render
<ReferenceDetail {item}/> (or the snippet) for selectedNode.outgoing and
selectedNode.incoming to eliminate duplication and keep logic centralized.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
infrastructure/control-panel/.npmrcinfrastructure/control-panel/package.jsoninfrastructure/control-panel/src/routes/+layout.svelteinfrastructure/control-panel/src/routes/api/references/+server.tsinfrastructure/control-panel/src/routes/visualizer/+page.server.tsinfrastructure/control-panel/src/routes/visualizer/+page.svelteplatforms/ereputation/api/src/controllers/ReferenceController.tsplatforms/ereputation/api/src/database/seed.tsplatforms/ereputation/api/src/index.tsplatforms/ereputation/api/src/services/ReferenceService.ts
infrastructure/control-panel/src/routes/api/references/+server.ts
Outdated
Show resolved
Hide resolved
infrastructure/control-panel/src/routes/api/references/+server.ts
Outdated
Show resolved
Hide resolved
infrastructure/control-panel/src/routes/visualizer/+page.svelte
Outdated
Show resolved
Hide resolved
platforms/ereputation/api/src/controllers/ReferenceController.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (8)
infrastructure/control-panel/src/routes/visualizer/+page.svelte (3)
64-64: PreviousnumericScorefalsiness concern addressed inbuildGraph.Line 64 now correctly uses
!= null. Good fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/control-panel/src/routes/visualizer/`+page.svelte at line 64, The falsy-check for numericScore was corrected: ensure the scoreLabel computation uses a null/undefined-only check (ref.numericScore != null) so you don't erroneously reject valid values like 0; update or keep the expression that sets scoreLabel (const scoreLabel = ref.numericScore != null ? ` (${ref.numericScore}/5)` : '') and verify related logic in buildGraph still treats numericScore the same way.
177-179: Previous resource-leak concern addressed.The
onMountcallback now returns a cleanup function that callsnetwork.destroy(), properly releasing the vis-network instance on component teardown.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/control-panel/src/routes/visualizer/`+page.svelte around lines 177 - 179, The cleanup currently calls network.destroy() unconditionally; update the onMount teardown to guard and nullify the instance by checking the vis-network variable (e.g., network) exists before calling destroy and then set it to null to avoid potential runtime errors and lingering references — e.g., replace the cleanup with a conditional call like if (network) network.destroy(); network = null;.
281-281:⚠️ Potential issue | 🟡 Minor
numericScoreof0is still hidden in the template sections.The
buildGraphfix on Line 64 was applied, but the template{#ifitem.ref.numericScore}checks on Lines 281 and 343 still use truthiness — a score of0would be suppressed.Proposed fix (apply to both occurrences)
-{`#if` item.ref.numericScore} +{`#if` item.ref.numericScore != null}Also applies to: 343-343
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/control-panel/src/routes/visualizer/`+page.svelte at line 281, The template currently hides a numericScore of 0 because it uses truthiness checks; update both occurrences that use "{`#if` item.ref.numericScore}" to explicitly test for presence (e.g., check that item.ref.numericScore is not null/undefined or is a number) so 0 is rendered; locate the conditional usages referencing item.ref.numericScore in the +page.svelte template (both occurrences around the existing checks) and replace the truthy check with an explicit null/undefined (or typeof number) check.infrastructure/control-panel/.npmrc (1)
1-2: Previous documentation concern addressed.The comment on Line 1 explains why
engine-strictwas relaxed. This addresses the prior review feedback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/control-panel/.npmrc` around lines 1 - 2, The file relaxes engine enforcement by setting "engine-strict=false" and documents the reason with the comment "vis-network required node < 22"; leave the comment and the engine-strict=false setting as-is (no code change needed), or if you prefer clearer intent, expand the comment to explicitly state which dependency requires Node <22 and/or add a link to the upstream issue, but do not remove the comment or revert the engine-strict change.platforms/ereputation/api/src/database/seed.ts (2)
43-43: Previoussynchronize: trueconcern addressed.Now correctly set to
synchronize: false, preventing unintended schema alterations on seed runs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/ereputation/api/src/database/seed.ts` at line 43, The config change correctly sets synchronize: false in the database connection configuration (seed.ts); ensure no leftover duplicate review markers remain by removing the duplicate comment tag and, if you intend to apply schema changes during seeding, wire in running migrations instead of relying on synchronize (e.g., call your migration runner before seeding); reference the synchronize property in seed.ts and any migration runner function you use to implement this.
149-155: PreviousrefRepo.clear()concern addressed.The deletion is now properly scoped to seed user IDs using
In(seedUserIds), preserving any non-seed reference data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/ereputation/api/src/database/seed.ts` around lines 149 - 155, The deletion was correctly scoped to seed users using savedUsers.map(...) and refRepo.delete({ authorId: In(seedUserIds) }), so keep that change but remove the duplicate review metadata/comment markers (e.g., "[duplicate_comment]" / "[approve_code_changes]") from the PR comment; also add a quick guard to ensure seedUserIds.length > 0 before calling refRepo.count/delete so refRepo.delete is only invoked when there are seed ids.platforms/ereputation/api/src/controllers/ReferenceController.ts (1)
99-103: Previous null-guard concern addressed.The
ref.authorternary now safely handles orphaned references, matching the optional-chaining pattern used elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/ereputation/api/src/controllers/ReferenceController.ts` around lines 99 - 103, The null-guard for ref.author is correctly implemented—retain the ternary that maps author: ref.author ? { id: ref.author.id, ename: ref.author.ename, name: ref.author.name } : null and ensure it remains consistent with the optional-chaining pattern used elsewhere (e.g., other uses of ref.author?.property); no functional change required beyond keeping this explicit null-handling in the author mapping block.infrastructure/control-panel/src/routes/api/references/+server.ts (1)
6-6: PreviousVITE_prefix concern addressed.Now correctly using
env.EREPUTATION_BASE_URL, keeping the backend URL server-only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/control-panel/src/routes/api/references/`+server.ts at line 6, The constant baseUrl is now correctly assigned from env.EREPUTATION_BASE_URL but you should ensure the env var name is correct and kept server-only, validate/normalize the resulting URL before use (e.g., ensure it has a scheme/port) and avoid ever exporting or embedding baseUrl into client bundles; update any remaining references that previously used VITE_ prefixed values to read from the baseUrl constant so everything consistently uses env.EREPUTATION_BASE_URL.
🧹 Nitpick comments (1)
infrastructure/control-panel/src/routes/visualizer/+page.svelte (1)
229-233:slidetransitions on the<aside>are ineffective — the element is always in the DOM.Svelte
in:/out:transitions only fire when the element is inserted into or removed from the DOM. Since the<aside>is unconditionally rendered (the{#ifselectedNode}is inside it), the slide transition will only trigger on initial page mount, not when a node is selected/deselected.If you want the panel to animate in/out, wrap the
<aside>(or its content) in a conditional block:♻️ Sketch
- <aside - class="flex w-80 shrink-0 flex-col gap-3 overflow-y-auto rounded-lg border border-gray-200 bg-white p-5" - in:slide={{ axis: 'x', duration: 150 }} - out:slide={{ axis: 'x', duration: 150 }} - > - {`#if` selectedNode} + {`#if` selectedNode} + <aside + class="flex w-80 shrink-0 flex-col gap-3 overflow-y-auto rounded-lg border border-gray-200 bg-white p-5" + in:slide={{ axis: 'x', duration: 150 }} + out:slide={{ axis: 'x', duration: 150 }} + > {`#key` selectedNode.id} <!-- ... panel content ... --> {/key} - {:else} - <p class="py-4 text-center text-sm text-gray-400"> - Click a node to see details about its references. - </p> - {/if} - </aside> + </aside> + {/if}You could keep a static placeholder outside the
{#if}for the "click a node" prompt if desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrastructure/control-panel/src/routes/visualizer/`+page.svelte around lines 229 - 233, The <aside> currently has in:slide/out:slide but remains always in the DOM so transitions only run on mount; change the rendering so the conditional that checks selectedNode surrounds the <aside> (or the inner content you want animated) instead of being inside it — e.g., move the {`#if` selectedNode} block to wrap the <aside> (or wrap the panel content) so Svelte can insert/remove the element and trigger the in:slide/out:slide transitions on selection/deselection.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infrastructure/control-panel/src/routes/api/references/`+server.ts:
- Around line 9-13: The AbortController created as controller is never passed
into fetch, so the 5s timeout is a no-op and setTimeout isn't cleared on error;
update the fetch call that calls `${baseUrl}/api/references/all` to include the
controller.signal in its options (e.g., add signal: controller.signal alongside
headers) and ensure clearTimeout(timeout) is invoked in all exit paths (both
after a successful response and before throwing when !response.ok) so the timer
is always cleared.
In `@infrastructure/control-panel/src/routes/visualizer/`+page.svelte:
- Around line 37-46: The loop that builds author nodes uses ref.author.id and
ref.author.name without a null check and will throw for orphaned references;
update the loop in the visualizer (+page.svelte) to guard ref.author (e.g., if
(!ref.author) { create a fallback author node with a stable id/label like
`unknown:${ref.id}` or skip adding an author) before computing authorKey and
nodeMap.set; ensure the code paths that reference authorKey, label, and type
(the nodeMap logic) work with the chosen fallback so the visualizer does not
crash when getAllReferences returns a null author.
In `@platforms/ereputation/api/src/controllers/ReferenceController.ts`:
- Around line 81-84: The current API-key guard (checking
process.env.VISUALIZER_API_KEY before validating
req.headers['x-visualizer-key']) silently allows unauthenticated access when
VISUALIZER_API_KEY is unset; change the behavior to "fail closed" by requiring
the env var and returning an error if it's missing or if the header does not
match: ensure the code that reads process.env.VISUALIZER_API_KEY validates it
exists and returns a 500/401 (or other appropriate error) when absent, and
otherwise compares req.headers['x-visualizer-key'] to VISUALIZER_API_KEY and
returns res.status(401).json({ error: 'Unauthorized' }) on mismatch so the
endpoint cannot be publicly accessed when the key is not configured.
- Around line 80-87: The controller's getAllReferences handler currently calls
this.referenceService.getAllReferences() with no parameters which relies on a
500-item default and therefore silently truncates results; update
ReferenceController.getAllReferences to accept pagination from the request (read
req.query.limit and req.query.offset, parse/validate to integers with safe
defaults) and pass them through to this.referenceService.getAllReferences(limit,
offset) so callers can paginate, or alternatively implement server-side
pagination inside the same handler by repeatedly calling
this.referenceService.getAllReferences(limit, offset) in a loop (advancing
offset) until fewer than limit results are returned and then return the
accumulated array; ensure you reference the methods getAllReferences on
ReferenceController and referenceService and validate query inputs to avoid DoS.
---
Duplicate comments:
In `@infrastructure/control-panel/.npmrc`:
- Around line 1-2: The file relaxes engine enforcement by setting
"engine-strict=false" and documents the reason with the comment "vis-network
required node < 22"; leave the comment and the engine-strict=false setting as-is
(no code change needed), or if you prefer clearer intent, expand the comment to
explicitly state which dependency requires Node <22 and/or add a link to the
upstream issue, but do not remove the comment or revert the engine-strict
change.
In `@infrastructure/control-panel/src/routes/api/references/`+server.ts:
- Line 6: The constant baseUrl is now correctly assigned from
env.EREPUTATION_BASE_URL but you should ensure the env var name is correct and
kept server-only, validate/normalize the resulting URL before use (e.g., ensure
it has a scheme/port) and avoid ever exporting or embedding baseUrl into client
bundles; update any remaining references that previously used VITE_ prefixed
values to read from the baseUrl constant so everything consistently uses
env.EREPUTATION_BASE_URL.
In `@infrastructure/control-panel/src/routes/visualizer/`+page.svelte:
- Line 64: The falsy-check for numericScore was corrected: ensure the scoreLabel
computation uses a null/undefined-only check (ref.numericScore != null) so you
don't erroneously reject valid values like 0; update or keep the expression that
sets scoreLabel (const scoreLabel = ref.numericScore != null ? `
(${ref.numericScore}/5)` : '') and verify related logic in buildGraph still
treats numericScore the same way.
- Around line 177-179: The cleanup currently calls network.destroy()
unconditionally; update the onMount teardown to guard and nullify the instance
by checking the vis-network variable (e.g., network) exists before calling
destroy and then set it to null to avoid potential runtime errors and lingering
references — e.g., replace the cleanup with a conditional call like if (network)
network.destroy(); network = null;.
- Line 281: The template currently hides a numericScore of 0 because it uses
truthiness checks; update both occurrences that use "{`#if`
item.ref.numericScore}" to explicitly test for presence (e.g., check that
item.ref.numericScore is not null/undefined or is a number) so 0 is rendered;
locate the conditional usages referencing item.ref.numericScore in the
+page.svelte template (both occurrences around the existing checks) and replace
the truthy check with an explicit null/undefined (or typeof number) check.
In `@platforms/ereputation/api/src/controllers/ReferenceController.ts`:
- Around line 99-103: The null-guard for ref.author is correctly
implemented—retain the ternary that maps author: ref.author ? { id:
ref.author.id, ename: ref.author.ename, name: ref.author.name } : null and
ensure it remains consistent with the optional-chaining pattern used elsewhere
(e.g., other uses of ref.author?.property); no functional change required beyond
keeping this explicit null-handling in the author mapping block.
In `@platforms/ereputation/api/src/database/seed.ts`:
- Line 43: The config change correctly sets synchronize: false in the database
connection configuration (seed.ts); ensure no leftover duplicate review markers
remain by removing the duplicate comment tag and, if you intend to apply schema
changes during seeding, wire in running migrations instead of relying on
synchronize (e.g., call your migration runner before seeding); reference the
synchronize property in seed.ts and any migration runner function you use to
implement this.
- Around line 149-155: The deletion was correctly scoped to seed users using
savedUsers.map(...) and refRepo.delete({ authorId: In(seedUserIds) }), so keep
that change but remove the duplicate review metadata/comment markers (e.g.,
"[duplicate_comment]" / "[approve_code_changes]") from the PR comment; also add
a quick guard to ensure seedUserIds.length > 0 before calling
refRepo.count/delete so refRepo.delete is only invoked when there are seed ids.
---
Nitpick comments:
In `@infrastructure/control-panel/src/routes/visualizer/`+page.svelte:
- Around line 229-233: The <aside> currently has in:slide/out:slide but remains
always in the DOM so transitions only run on mount; change the rendering so the
conditional that checks selectedNode surrounds the <aside> (or the inner content
you want animated) instead of being inside it — e.g., move the {`#if`
selectedNode} block to wrap the <aside> (or wrap the panel content) so Svelte
can insert/remove the element and trigger the in:slide/out:slide transitions on
selection/deselection.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
infrastructure/control-panel/.npmrcinfrastructure/control-panel/src/routes/api/references/+server.tsinfrastructure/control-panel/src/routes/visualizer/+page.svelteplatforms/blabsy/next-env.d.tsplatforms/ereputation/api/src/controllers/ReferenceController.tsplatforms/ereputation/api/src/database/seed.tsplatforms/ereputation/api/src/services/ReferenceService.ts
✅ Files skipped from review due to trivial changes (1)
- platforms/blabsy/next-env.d.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- platforms/ereputation/api/src/services/ReferenceService.ts
platforms/ereputation/api/src/controllers/ReferenceController.ts
Outdated
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of change
eReference Network Visualizer
Added a force-directed graph at
/visualizerin the control panel showing all signed eReferences across users, groups and platforms.What was added:
GET /api/references/all— new public endpoint in the eReputation API returning all signed referencesplatforms/ereputation/api/src/database/seed.ts— populates 15 users and ~60 signed references for local testing/visualizerroute in the control panel — interactive vis-network graph with node degree sizing, click-to-inspect sidebar (outgoing/incoming references per node in collapsible accordions)To seed local data:
```sh
cd platforms/ereputation/api
npx ts-node src/database/seed.ts
```
To run with seed data:
```sh
.env — keep this pointing to local:
VITE_EREPUTATION_BASE_URL='http://localhost:8765'
VISUALIZER_API_KEY="dev-visualizer-api-key"
Then start both servers:
cd platforms/ereputation/api && pnpm dev
cd infrastructure/control-panel && pnpm dev
```
Once this PR is merged and deployed, switch
VITE_EREPUTATION_BASE_URLto the production URL to use real data.Issue Number
Closes #784
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
Chores