Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughIntroduces CostLens, a complete full-stack SaaS cost analysis platform featuring a React frontend dashboard, Express backend server with multi-pillar cost analysis endpoints (infrastructure, build, buyer, risk, competitor), TinyFish web agent integration for data collection, OpenAI-powered AI cost modeling, and configuration for local development and Vercel deployment. Changes
Sequence DiagramssequenceDiagram
participant Client as Client<br/>(React)
participant Server as Server<br/>(Express)
participant TinyFish as TinyFish<br/>Web Agent
participant OpenAI as OpenAI<br/>API
participant LocalStorage as LocalStorage
Client->>Server: POST /api/investigate/async<br/>(url)
activate Server
Server->>TinyFish: runAsync (infra, build, buyer, risk, competitors)
activate TinyFish
TinyFish-->>Server: runIds[] (5 parallel runs)
deactivate TinyFish
Server-->>Client: { runIds, status }
deactivate Server
loop Poll Results
Client->>Server: POST /api/investigate/async/poll<br/>(runIds)
activate Server
Server->>TinyFish: getRun(runId) [×5]
activate TinyFish
TinyFish-->>Server: results[] (when complete)
deactivate TinyFish
Server->>OpenAI: synthesize costs<br/>(infra, build, buyer)
activate OpenAI
OpenAI-->>Server: costModels + executive
deactivate OpenAI
Server-->>Client: { report, quality }
deactivate Server
end
Client->>LocalStorage: saveReport()
Client->>Client: display views<br/>(infra, build, buyer, risk)
sequenceDiagram
participant Client as Client<br/>(React)
participant Server as Server<br/>(Express)
participant TinyFish as TinyFish<br/>Web Agent
participant OpenAI as OpenAI<br/>API
Client->>Server: GET /api/investigate/stream<br/>(url)
activate Server
Server->>Client: SSE stream open
Server->>TinyFish: runAutomation (5 pillars)
activate TinyFish
loop For Each Pillar
TinyFish-->>Server: SSE event
Server-->>Client: progress event
end
TinyFish-->>Server: complete
deactivate TinyFish
Server->>OpenAI: synthesize report
activate OpenAI
OpenAI-->>Server: complete report
deactivate OpenAI
Server-->>Client: final event + close
deactivate Server
Client->>Client: render results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR introduces a substantial, heterogeneous full-stack application spanning 50+ new files with diverse logic density. While individual components are modular, the complexity stems from: intricate investigation workflows in Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ 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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@costlens/client/src/components/ExecutiveSummary.jsx`:
- Line 21: The destructured props from executiveSummary (summary, keyFindings,
recommendations, verdictLabel) can be undefined so accessing keyFindings.length
and recommendations.length may throw; fix by defaulting them to safe values when
destructuring (e.g., destructure from executiveSummary || {} and set keyFindings
= [] and recommendations = []) and/or add guards before using .length so the
component (ExecutiveSummary.jsx) always operates on arrays (use keyFindings = []
and recommendations = [] in the const { ... } assignment or equivalent
null-coalescing checks).
In `@costlens/client/src/components/NegotiationPlaybook.jsx`:
- Around line 8-10: Destructuring from negotiation can yield undefined
sub-properties and cause a TypeError when calling .length; update the
destructuring of negotiation (leverageFactors, talkingPoints, counterOffers,
riskWarnings) to default each to [] and include riskWarnings in the hasContent
check so the component safely handles missing backend fields and remains
consistent with its conditional rendering (look for the negotiation
destructuring and the hasContent variable).
In `@costlens/client/src/components/ReportSummary.jsx`:
- Around line 23-38: The map callback for checklistPillars uses a parameter
named pillar which shadows the outer pillar variable and is error-prone; update
the callback parameter name in checklistPillars.map (and the other .map at the
same file) to a non-conflicting identifier such as pillarKey or p, then update
all uses inside that callback (e.g., the pillar -> pillarKey references used to
build the returned object: label, status, details) so the outer pillar variable
is no longer shadowed.
In `@costlens/client/src/components/ui/ExpandableRow.jsx`:
- Around line 14-58: The ExpandableRow component currently renders subtitle and
expanded children inside a single <button>, which creates invalid nested
interactive content; refactor ExpandableRow so the <button> only contains the
clickable header (title, badge, titleRight and the expand icon) and move
subtitle and the expanded content (children) outside the button into a sibling
container (e.g., a <div> under the button); keep using the expanded prop and
onToggle handler on the header button, add an id on the content container and
set aria-controls on the button (and preserve aria-expanded and
accessibilityLabel), and ensure the expanded content is conditionally rendered
outside the button so nested interactive elements in children (like WhyHint) are
not placed inside a button.
In `@costlens/client/src/views/LandingView.jsx`:
- Line 43: The Enter key handler in LandingView.jsx currently calls runScan()
unconditionally, bypassing the button's disabled guard; update the onKeyDown
handler to only trigger runScan() when the same conditions as the button are met
(e.g., !scanning && url.trim()), and optionally call e.preventDefault() to avoid
duplicate submits. Locate the onKeyDown usage in LandingView.jsx and change its
logic so it checks scanning and url.trim() before invoking runScan() (and keep
using the existing runScan() function and scanning/url state variables).
In `@costlens/server/server.js`:
- Around line 363-453: Duplicate report-assembly logic (from failedPillars
calculation through the returned report object) exists in runInvestigation and
the poll endpoint; extract it into a shared assembleReport(...) function and
call it from both places. Create assembleReport({ report, executiveSummary,
negotiation, riskProfile, competitorAnalysis, scannerErrors, infraRaw, buildRaw,
buyerRaw, riskRaw, timedOut, config, name, domain }) that performs the
failedPillars/degradedPillars computation, builds pillarMeta via
normalizePillarMeta, computes qualityMeta via buildQualityMeta, calculates
legacyCompleteness, and returns the final report object exactly as in the diff
(including timeout merging into scannerErrors, provenance fields, and default
riskProfile using modeler.getDefaultRiskProfile() when needed); then replace the
duplicated blocks in runInvestigation and the poll endpoint with calls to
assembleReport, ensuring competitorAnalysis is optional so the sync path
behavior remains unchanged.
- Around line 663-824: The poll handler at
app.post("/api/investigate/async/poll") is re-running full AI synthesis
(CostModeler.analyze and subsequent modeler.generate* calls) on every completed
poll, causing duplicate expensive API calls; fix by introducing a short-lived
synthesis cache or status keyed by the runIds before calling CostModeler.
Specifically, before invoking CostModeler.analyze or
modeler.generateExecutiveSummary/generateNegotiationPlaybook/analyzeRiskProfile/generateCompetitorAnalysis,
check a shared store (e.g., an in-memory Map or short-lived KV) using a
deterministic key derived from runIds; if an entry exists return the cached
synthesized report (or a saved status like "synthesizing") instead of
re-running; when starting synthesis set the key to a "synthesizing" marker so
concurrent polls return status:"synthesizing", and once synthesis completes
store the final report under that key with an expiration TTL to avoid stale
data. Ensure you reference the runIds object and store the final response body
structure you currently send so subsequent polls return identical JSON.
- Around line 60-65: The current Vercel check (isVercelDeployment +
/^https:\/\/[a-z0-9-]+\.vercel\.app$/i.test(origin)) is too permissive; narrow
it to only your project's Vercel host or make it configurable. Replace the
blanket regex with a project-specific pattern (for example require the subdomain
to start with "costlens-" such as
/^https:\/\/costlens-[a-z0-9-]+\.vercel\.app$/i.test(origin)) or read an allowed
Vercel hostname/pattern from an env var (e.g., ALLOWED_VERCEL_HOST or
VERCEL_PROJECT_PREFIX) and validate origin against that value, keeping the
existing isVercelDeployment and callback logic in the same place.
In `@costlens/server/tinyfish/tinyfish-web-agent-client.js`:
- Around line 95-153: The SSE stream in _runSse can hang because sseTimeoutMs
only applies to the initial _fetchWithTimeout; fix by wiring an AbortController
into the streaming phase: create an AbortController before calling
_fetchWithTimeout (or modify _fetchWithTimeout to accept a signal) and pass its
signal to fetch, then start a timeout timer for sseTimeoutMs that calls
controller.abort() if no progress; inside the reader loop reset or refresh that
timer each time you successfully decode a chunk (so active streams don’t time
out) and clear the timer/close the reader on completion or error; ensure abort
errors are caught and turned into a clear timeout Error rather than leaving
reader.read() hung.
🟡 Minor comments (20)
costlens/client/src/components/InfraVisualizations.jsx-33-38 (1)
33-38:⚠️ Potential issue | 🟡 Minor
itemsis accessed without a null guard.If
itemsisundefinedornull, the.filter()call on Line 38 (and Line 60) will throw. Consider a default parameter:-export function CostBar({ items }) { +export function CostBar({ items = [] }) {costlens/client/src/components/InfraVisualizations.jsx-21-27 (1)
21-27:⚠️ Potential issue | 🟡 MinorNo defensive handling if
low,mid, orhighareundefined/NaN.
mid.toFixed(0)(Line 21),low.toFixed(1)andhigh.toFixed(1)(Line 27) will throw aTypeErrorif the parent passesundefinedornull. Consider defaulting props or adding a guard, e.g.:-export function MarginGauge({ low, mid, high }) { +export function MarginGauge({ low = 0, mid = 0, high = 0 }) {costlens/package.json-17-25 (1)
17-25:⚠️ Potential issue | 🟡 MinorMove
reactandreact-domtodevDependencies.This project uses a client/server architecture: the client is a Vite-built React SPA, and the server is a pure Express API that serves static assets and endpoints. React is only needed at build time to compile the client; the Express server does not require it at runtime.
♻️ Move to devDependencies
"dependencies": { "cors": "^2.8.5", "dotenv": "^17.3.1", "express": "^4.18.2", "express-rate-limit": "^8.2.1", - "openai": "^4.28.0", - "react": "^18.2.0", - "react-dom": "^18.2.0" + "openai": "^4.28.0" }, "devDependencies": { "@vitejs/plugin-react": "^4.2.0", + "react": "^18.2.0", + "react-dom": "^18.2.0", "supertest": "^7.0.0", "vite": "^5.0.0" }costlens/server/services/infra-cost-scanner.js-15-15 (1)
15-15:⚠️ Potential issue | 🟡 MinorMisleading comment: "Fast: 2 runs" mode doesn't exist in the code.
The comment says "Ultra-fast: 1 combined run for infra (tech + traffic). Fast: 2 runs. Full: 4 runs." but the code only implements two modes:
fast(1 run) and full (4 runs). There is no 2-run mode.costlens/server/services/infra-cost-scanner.js-50-71 (1)
50-71:⚠️ Potential issue | 🟡 Minor
raw.techStack ?? rawmay produce confusing results when the AI doesn't nest undertechStack.On Line 68, if
response.resultis a flat object without atechStackkey, the entire result gets assigned astechStack. This could make downstream consumers receive traffic data mixed into thetechStackfield. Consider falling back tonullor{}instead.- techStack: raw.techStack ?? raw, + techStack: raw.techStack ?? null,costlens/client/src/utils/export.js-44-48 (1)
44-48:⚠️ Potential issue | 🟡 MinorBiome lint:
forEachcallback should not return a value.The arrow shorthand
(f) => lines.push(...)implicitly returns the result ofpush. Use a block body to silence the lint rule. Same pattern appears on lines 79, 100.Proposed fix (apply similarly to lines 79 and 100)
- report.executiveSummary.keyFindings.forEach((f) => lines.push(` • ${f}`)); + report.executiveSummary.keyFindings.forEach((f) => { lines.push(` • ${f}`); });costlens/server/config/index.js-4-5 (1)
4-5:⚠️ Potential issue | 🟡 MinorCORS default may block the Vite dev client.
corsOrigindefaults tohttp://localhost:3000, which is also the server's default port. Vite typically serves on port 5173, so local development requests from the client will be blocked by CORS unlessCORS_ORIGINis explicitly set.Suggested fix
- corsOrigin: process.env.CORS_ORIGIN || "http://localhost:3000", + corsOrigin: process.env.CORS_ORIGIN || "http://localhost:5173",costlens/client/src/components/AppHeader.jsx-106-108 (1)
106-108:⚠️ Potential issue | 🟡 Minor
degradedSetanddegradedReasonnot guarded against undefined.If the parent omits these props,
degradedSet.has(id)throws. Consider defaulting in the destructuring.Proposed fix
-export function AppHeader({ hasResults, view, onViewChange, onGoHome, tabMeta, degradedSet, degradedReason }) { +export function AppHeader({ hasResults, view, onViewChange, onGoHome, tabMeta, degradedSet = new Set(), degradedReason = () => "" }) {costlens/client/src/views/RiskView.jsx-59-59 (1)
59-59:⚠️ Potential issue | 🟡 Minor
ScoreGaugemay render a misleading "0" when no security score exists.
hasMeaningfulDataon line 59 istruewhenfindings.length > 0 || badges.length > 0, even ifsecurityScoreis0. In that caseScoreGaugerenders a gauge showing "0", which looks like a measured zero rather than "no data." Consider gating the gauge onrisk.securityScore > 0separately.🔧 Proposed fix
<Panel style={{ padding: 24, display: "flex", flexDirection: "column", alignItems: "center", justifyContent: "center" }}> - {hasMeaningfulData ? ( + {risk.securityScore > 0 ? ( <ScoreGauge score={risk.securityScore} label="Security Score" /> ) : ( <div style={{ fontFamily: "'IBM Plex Mono',monospace", fontSize: type.sizeSm, color: colors.textMuted, textAlign: "center" }}> Insufficient data for security score </div> )} </Panel>Also applies to: 82-90
costlens/client/src/components/ScanHistory.jsx-91-107 (1)
91-107:⚠️ Potential issue | 🟡 MinorMissing
Spacekey handler on the delete control.
role="button"elements should activate on both Enter and Space per ARIA authoring practices. Currently only Enter is handled, so keyboard users pressing Space will scroll the page instead of deleting.🔧 Proposed fix
- onKeyDown={(e) => { if (e.key === "Enter") handleDelete(entry.id, e); }} + onKeyDown={(e) => { if (e.key === "Enter" || e.key === " ") { e.preventDefault(); handleDelete(entry.id, e); } }}costlens/server/scripts/smoke-report-shape.js-16-24 (1)
16-24:⚠️ Potential issue | 🟡 MinorUnsafe
buyer.plans[0]access — crashes if default plans array is empty.Line 21 accesses
buyer.plans[0].namewithout a length guard. IfgetDefaultBuyerCost()ever returns an emptyplansarray (or the normalization changes), this will throw aTypeErrorat runtime. The assertion on Line 18 only checksArray.isArray, not length.🛡️ Proposed fix: add a length assertion before index access
assert(Array.isArray(buyer.plans), "Buyer plans should always be an array"); + assert(buyer.plans.length > 0, "Buyer default plans should contain at least one entry"); assert(typeof infra.monthlyEstimate.low === "number", "Infra monthly low should be numeric"); assert(typeof build.teamSize.min === "number", "Build team min should be numeric"); assert(typeof buyer.plans[0].name === "string", "Buyer plan name should be string");costlens/server/test/api.test.js-33-38 (1)
33-38:⚠️ Potential issue | 🟡 MinorAccepting HTTP 500 weakens the test.
Allowing
500alongside200and400on line 35 means an internal server crash is silently accepted. If the intent is to handle missing env vars gracefully, the server should return400with amissingEnvpayload — not500. Consider tightening this to[200, 400]and treating500as a test failure.🔧 Proposed fix
- assert.ok([200, 400, 500].includes(validUrlRes.status), "POST with valid URL should return 200, 400, or 500 depending on env and providers"); + assert.ok([200, 400].includes(validUrlRes.status), "POST with valid URL should return 200 or 400 (500 indicates an unhandled error)");costlens/client/src/utils/report.js-351-351 (1)
351-351:⚠️ Potential issue | 🟡 Minor
completenessScoredefaults to100when missing — should likely be0.If the server omits
completenessScore, the report silently appears fully complete. A default of0is the safer conservative fallback, consistent with how the other confidence/quality fields default to their "low" / zero values.🔧 Proposed fix
- completenessScore: toNum(results?.quality?.completenessScore, 100), + completenessScore: toNum(results?.quality?.completenessScore, 0),Also update the fallback object on line 68:
- completenessScore: 100, + completenessScore: 0,costlens/server/server.js-83-92 (1)
83-92:⚠️ Potential issue | 🟡 MinorHealth endpoint exposes missing environment variable names.
missingEnv(Line 90) reveals which API keys are absent. In production, this helps attackers understand the system's dependencies. Consider omitting the names and only returning the booleanenvReady.costlens/client/src/views/BuyerView.jsx-68-68 (1)
68-68:⚠️ Potential issue | 🟡 MinorUnguarded property accesses may throw if normalization produces unexpected shapes.
Several
.lengthaccesses assume arrays are always present without optional chaining, while earlier lines (e.g., Line 10, 15, 20) defensively use?.. If a plan lackshiddenCostsorgotchas, or ifreport.buyerCost.plans/tcoComparison/competitorComparisonisundefined, these lines will throw aTypeError.Examples:
report.buyerCost.plans.length(Line 68, 82),plan.hiddenCosts.length(Line 97),plan.gotchas.length(Line 114),report.buyerCost.tcoComparison.length(Line 128),report.buyerCost.competitorComparison.length(Line 141).Consider using optional chaining consistently, e.g.
report.buyerCost?.plans?.lengthandplan.hiddenCosts?.length, or add fallbacks like(plan.hiddenCosts || []).Also applies to: 82-82, 97-97, 114-114, 128-128, 141-141
costlens/server/services/build-cost-estimator.js-44-56 (1)
44-56:⚠️ Potential issue | 🟡 Minor
domain.split(".")[0]may extract a subdomain prefix instead of the company name.For URLs like
app.example.com,hostname.split(".")[0]yields"app", not"example". This affects the TinyFish goal prompt. This is a known limitation shared with similar logic inserver.js(Line 255) andgetHiringCosts(Line 60).costlens/server/server.js-906-911 (1)
906-911:⚠️ Potential issue | 🟡 MinorError handler leaks allowed origins in CORS error responses.
originHint(Line 908) exposes the configured allowed origins to the caller. This reveals internal deployment URLs to potential attackers probing for CORS misconfigurations.Proposed fix — omit the hint
- const originHint = [...getAllowedProdOrigins()]; - if (status < 500 && originHint.length > 0 && error?.message?.includes("CORS")) { - payload.allowedOrigins = originHint; - }costlens/server/services/build-cost-estimator.js-19-23 (1)
19-23:⚠️ Potential issue | 🟡 MinorType mismatch:
featuresfallback is[]but expected shape is an object.When
detectFeaturessucceeds it returns{ detected: [...], pricingPageFeatures: [...] }. On failure, Line 20 falls back to[](an empty array). Downstream code (e.g.,result.features.detectedinbuildMetaat Line 81, and the cost modeler's prompt serialization) expects an object, not an array. While this doesn't crash (optional chaining inbuildMetacatches it), the semantic mismatch means metadata will silently report no features even if partial data was available.Proposed fix
const result = { - features: results[0]?.status === "fulfilled" ? results[0].value : [], + features: results[0]?.status === "fulfilled" ? results[0].value : { detected: [], pricingPageFeatures: [] }, openSource: !fast && results[1]?.status === "fulfilled" ? results[1].value : [], hiring: !fast && results[2]?.status === "fulfilled" ? results[2].value : null, };costlens/client/src/views/CompetitorView.jsx-38-68 (1)
38-68:⚠️ Potential issue | 🟡 MinorCompetitor dots access
c.positioningwithout a guard.
c.positioning.featureRichnessandc.positioning.priceLevel(Lines 39-40) will throw if a competitor object is missing thepositioningproperty. While the normalizer currently guarantees it, a defensive default would be safer:- const cx = toX(c.positioning.featureRichness); - const cy = toY(c.positioning.priceLevel); + const cx = toX(c.positioning?.featureRichness ?? 3); + const cy = toY(c.positioning?.priceLevel ?? 3);costlens/server/analysis/cost-modeler.js-137-141 (1)
137-141:⚠️ Potential issue | 🟡 Minor
reviewInsightsfallback is{}but an array is expected.Line 139 falls back to
{}forreviewInsights, but the data comes fromBuyerCostAnalyzer.scan()which returns it as an array. The system prompt will serialize{}as"{}"instead of"[]", giving the model misleading context.Proposed fix
- Review Insights: ${JSON.stringify(data?.reviewInsights || {})} + Review Insights: ${JSON.stringify(data?.reviewInsights || [])}
🧹 Nitpick comments (32)
costlens/client/src/components/InfraVisualizations.jsx (2)
1-31: Hardcoded color values duplicate design tokens.Colors like
"#C41E3A","#E8E4DF","#8B8680","#1A1815", and"#5C5650"are all defined incostlens/client/src/styles/tokens.js(accent,border,textMuted,text,textSecondary). Importing and referencingcolorsfrom tokens would keep the palette consistent and easier to update.♻️ Suggested import
+import { colors } from "../styles/tokens"; + export function MarginGauge({ low, mid, high }) {Then replace literals like
"#C41E3A"→colors.accent,"#E8E4DF"→colors.border, etc.
38-66: Filter is applied twice on the same array.
items.filter((i) => i.pct > 0)is evaluated identically on Lines 38 and 60. Extracting it to a variable avoids the duplication and extra iteration.♻️ Suggested refactor
export function CostBar({ items = [] }) { const colors = ["#C41E3A", "#D4574A", "#E8845C", "#F2A86B", "#F5C97A", "#8B7355", "#6B8F71", "#4A7A8C"]; + const visible = items.filter((i) => i.pct > 0); return ( <div> <div style={{ display: "flex", height: 28, borderRadius: 4, overflow: "hidden", gap: 1, marginBottom: 10 }}> - {items.filter((i) => i.pct > 0).map((item, i) => ( + {visible.map((item, i) => ( ... ))} </div> <div style={{ display: "flex", flexWrap: "wrap", gap: 8 }}> - {items.filter((i) => i.pct > 0).map((item, i) => ( + {visible.map((item, i) => ( ... ))} </div>costlens/package.json (1)
1-31: Nopackage-lock.jsonor lockfile visible in this PR.For reproducible builds, especially with all
^ranges, consider committing a lockfile. This is less critical for a cookbook/demo project but good practice.costlens/vercel.json (1)
7-7: Health endpointmaxDurationof 120s is unnecessarily generous.A health check should respond in milliseconds. Consider lowering to something like 10–15s to match its expected behavior and avoid masking slowness.
Proposed fix
- "api/health.js": { "maxDuration": 120 }, + "api/health.js": { "maxDuration": 10 },costlens/README.md (1)
93-96: Add a language specifier to the fenced code block.The markdownlint MD040 rule flags this block for missing a language identifier. Adding one (e.g.,
iniorbash) improves rendering and accessibility.Proposed fix
-``` +```ini TINYFISH_API_KEY=your_tinyfish_api_key OPENAI_API_KEY=your_openai_api_key ```costlens/client/src/components/ui/WhyHint.jsx (2)
61-84: Consider adding ARIA attributes to the popup for better screen reader support.The popup
<div>lacks aroleandidlinkage. Addingrole="tooltip"(or"status") with anid, and linking it from the button viaaria-describedby, would let assistive technologies announce the popup content.
10-15: Resize listener fires on every pixel — consider debouncing.
setIsMobileis called on every resize event. While React batches updates, a lightweight debounce ormatchMedialistener would be more efficient and idiomatic.matchMedia alternative
useEffect(() => { - const updateIsMobile = () => setIsMobile(window.innerWidth < 768); - updateIsMobile(); - window.addEventListener("resize", updateIsMobile); - return () => window.removeEventListener("resize", updateIsMobile); + const mql = window.matchMedia("(max-width: 767px)"); + setIsMobile(mql.matches); + const handler = (e) => setIsMobile(e.matches); + mql.addEventListener("change", handler); + return () => mql.removeEventListener("change", handler); }, []);costlens/server/tinyfish/tinyfish-web-agent-client.js (2)
28-52: Consider extracting API-key validation and payload construction to reduce duplication.
runAutomation,runSync, andrunAsyncall repeat the same API key check and near-identical payload construction (Lines 29-37, 60-68, 73-81). A small private helper would eliminate this repetition.
255-259: 400 (Bad Request) and 422 (Unprocessable Entity) are retried, which is typically wasteful.
_isRetryableonly excludes 401, 403, and 404. Client errors like 400 and 422 are deterministic and won't succeed on retry. Consider also returningfalsefor the 4xx range (except 408/429).Suggested fix
_isRetryable(error) { const statusCode = Number(error?.statusCode); if (statusCode === 401 || statusCode === 403 || statusCode === 404) return false; + if (statusCode === 400 || statusCode === 422) return false; return true; }costlens/server/services/infra-cost-scanner.js (1)
50-143: URL normalization logic is duplicated across multiple methods.The pattern
url.startsWith("http") ? url : \https://${url}`andnew URL(…).hostnameappears indetectTechStackAndTraffic,estimateTraffic,getEngineeringHeadcount, anddetectThirdPartyServices(implicitly via the passed URL). Consider extracting a small_normalizeUrl(url)` helper.costlens/server/services/buyer-cost-analyzer.js (1)
91-94:_coerceObjectsignature differs fromInfraCostScanner._coerceObject— consider unifying.Here it accepts a
fallbackparameter (Line 91), while ininfra-cost-scanner.jsit always returns{}. If these scanners share a common pattern, a shared utility would reduce drift.costlens/client/src/components/ui/SectionLabel.jsx (1)
12-12: Avoid arithmetic on design tokens.
space.sm + 2(= 10) breaks the token abstraction — if the spacing scale changes, this ad-hoc offset won't track. Consider usingspace.md(12) instead, or adding a dedicated token if 10px is intentional.costlens/client/src/components/ExportBar.jsx (2)
54-56: Usecolors.successtoken instead of hardcoded"#2E7D32".The success color is already defined in your tokens (
colors.success: "#2E7D32"), but Lines 54–56 hardcode it. This defeats the purpose of the token system.♻️ Proposed fix
- background: copied ? "#2E7D32" : colors.surface, - color: copied ? "#fff" : colors.textSecondary, - border: `1px solid ${copied ? "#2E7D32" : colors.border}`, + background: copied ? colors.success : colors.surface, + color: copied ? "#fff" : colors.textSecondary, + border: `1px solid ${copied ? colors.success : colors.border}`,
8-14: No user feedback on copy failure.If
copyTextSummaryreturnsfalse, the button silently does nothing. Consider a brief error indication (e.g., a "Failed" state) so users know the clipboard write didn't succeed.costlens/client/src/components/Badges.jsx (1)
1-49: Consider extracting shared badge styles and reusing tokens where applicable.Both badge components share identical inline style properties (padding, borderRadius, fontFamily, fontSize, fontWeight, letterSpacing). A shared base style object would reduce duplication. Additionally, some hardcoded colors like
"#2E7D32"and"#C41E3A"already exist ascolors.successandcolors.accentin your tokens.costlens/client/src/components/ScanOverlay.jsx (1)
18-18: Inline<style>tag re-injected on every render.The keyframe definitions are injected into the DOM each time the component renders. Consider extracting them to a shared CSS file or using a
useEffect/useInsertionEffectto inject once.costlens/client/src/utils/export.js (1)
118-121: Duplicatefmthelper — a different version already exists informatting.js.This file defines a local
fmtwithtoLocaleStringbehavior, whilecostlens/client/src/utils/formatting.jsexports anfmtwith$K/$Mshorthand. Having two divergent formatters namedfmtrisks confusion. Consider importing and reusing the shared one, or renaming this local variant (e.g.,fmtPlain) to clarify intent.costlens/client/src/components/ScanHistory.jsx (1)
31-46: Consider a confirmation step before clearing all history.
handleClearimmediately wipes all saved reports with no undo. Awindow.confirmor similar prompt would prevent accidental data loss.costlens/client/src/views/BuildView.jsx (1)
16-29: Inconsistent null-safety onreport.buildCostaccess.Lines 13–14 guard with
report.buildCost?.confidence, but lines 19 and 22–29 accessreport.buildCost.validationWarnings,report.buildCost.totalEstimate, etc. directly. Ifreportever arrives un-normalized (e.g., from a stale localStorage entry or future code path), these lines will throw.Pick one style: either trust
normalizeReportand drop the?.on lines 13–14, or add?.consistently everywhere. The latter is safer.costlens/server/test/api.test.js (1)
86-89: Test script may hang if the server keeps connections alive.
supertestcreates an ephemeral server, but ifappbinds listeners or timers internally, the Node process may not exit afterrun()completes. Consider callingprocess.exit(0)on success as well.🔧 Proposed fix
+ process.exit(0); } run().catch((err) => {costlens/client/src/utils/report.js (1)
353-500: Massive duplication — the per-pillar qualityMeta normalization is copy-pasted 5×.
pillarCoverage,sourceCoverage,dataFreshness,confidenceScore, andperPillareach repeat an identical block for infra/build/buyer/risk/competitors. This is ~150 lines that can collapse to ~30 with a helper + loop.♻️ Example refactor sketch
+const PILLARS = ["infra", "build", "buyer", "risk", "competitors"]; + +function normalizePillarMap(root, perKey) { + return Object.fromEntries(PILLARS.map((p) => [p, perKey(root?.[p])])); +} + // then in normalizeReport: - pillarCoverage: { - infra: { - tasksSucceeded: toNum(results?.quality?.qualityMeta?.pillarCoverage?.infra?.tasksSucceeded), - tasksExpected: toNum(results?.quality?.qualityMeta?.pillarCoverage?.infra?.tasksExpected), - }, - build: { /* ... identical ... */ }, - buyer: { /* ... identical ... */ }, - risk: { /* ... identical ... */ }, - competitors: { /* ... identical ... */ }, - }, + pillarCoverage: normalizePillarMap( + results?.quality?.qualityMeta?.pillarCoverage, + (v) => ({ tasksSucceeded: toNum(v?.tasksSucceeded), tasksExpected: toNum(v?.tasksExpected) }) + ),Apply the same pattern to
sourceCoverage,dataFreshness,confidenceScore, andperPillar.costlens/client/src/utils/history.js (1)
27-44: Storing full report blobs in localStorage risks hitting the ~5MB quota.Each
fullReportcan be deeply nested (infra, build, buyer, risk, competitor, quality, provenance). With 10 entries, serialized size could exceed the browser's localStorage limit. The silent catch inwriteStoremeans the save just disappears with no user feedback.Consider either compressing before storing, storing only the report id and re-fetching from the server, or at minimum surfacing a warning when the write fails.
costlens/client/src/views/InfraView.jsx (2)
132-148: Hardcoded border color#F0EDE8instead of design token.Line 139 uses a raw hex value for the signal divider while the rest of the component uses
colors.border(#E8E4DF). This may be intentional for a lighter separator, but it diverges from the token system and will be missed if the theme changes.🔧 Proposed fix
- borderBottom: i < signals.length - 1 ? "1px solid `#F0EDE8`" : "none", + borderBottom: i < signals.length - 1 ? `1px solid ${colors.border}` : "none",
13-23: Same optional chaining inconsistency as BuildView.Lines 14–15 access
report.infraCost.breakdownandreport.infraCost.signalsdirectly, while lines 17–18 guardreport.infraCost?.confidence. Be consistent — either trustnormalizeReportthroughout or use?.everywhere.costlens/client/src/App.jsx (3)
36-49: Dead code:actionsarray is never used.The
actionsarray defined here is never referenced anywhere in this component. The scan progress messages are set via hardcoded strings inrunScan(lines 92, 107, 127, 132, 143) and from server-side progress events — not from this array.Proposed fix — remove the unused array
- const actions = [ - "Fingerprinting tech stack via HTTP headers...", - "Extracting client-side JavaScript bundles...", - "Detecting cloud provider from CDN signatures...", - "Mapping API endpoints and database patterns...", - "Crawling Cloudflare Radar for traffic estimates...", - "Scanning LinkedIn for engineering headcount...", - "Extracting salary data from Glassdoor...", - "Analyzing pricing page for additional costs...", - "Cross-referencing G2 reviews for overage complaints...", - "Checking Crunchbase for revenue estimates...", - "Estimating infrastructure costs via AWS Calculator...", - "AI synthesizing cost model...", - ]; -
192-192:@importinside<style>blocks render progress.Loading Google Fonts via
@importinside a<style>tag is render-blocking — the browser must fetch the CSS before painting. Move this to a<link>tag in yourindex.htmlfor parallel loading.
55-81: Parameterurlshadows the component-level state variableurl.The
urlparameter on Line 55 shadows theurlstate from Line 25. This works correctly since the parameter is what's needed inside the function, but it could confuse future readers. Consider renaming the parameter torequestUrlorendpoint.costlens/client/src/views/CompetitorView.jsx (1)
236-298: Fixed two-column layout will break on narrow viewports.The
gridTemplateColumns: "1fr 1fr"at Line 236 combined with the fixedMAP_SIZE = 320means the map panel alone needs ~320 px + padding. On mobile or narrow windows the grid won't wrap, causing horizontal overflow.Consider using
repeat(auto-fit, minmax(340px, 1fr))or a media-query/container-query to stack the panels.costlens/client/src/components/ReportSummary.jsx (2)
10-10: Pillar-label mapping is duplicated.The
pillar → labelternary chain appears on both Line 10 and Line 32. Extract a small helper or lookup object to stay DRY:const PILLAR_LABELS = { infra: "Their Cost", build: "Build Cost", buyer: "Your Cost", risk: "Risk" };Also applies to: 32-32
40-47: Deeply nested ternary is hard to scan.Four levels of ternary nesting for
headlineTexthurts readability. A small lookup or switch would be clearer and easier to extend when new pillars are added.costlens/server/server.js (1)
309-311: Timeout rejects with a plain object instead of anError.
rej({ _timeout: true })creates an unhandled-rejection without a stack trace. Usenew Error("Investigation timeout")with a_timeoutproperty for better debuggability:- const timeoutP = new Promise((_, rej) => - setTimeout(() => rej({ _timeout: true }), timeoutMs) + const timeoutP = new Promise((_, rej) => + setTimeout(() => { const e = new Error("Investigation timeout"); e._timeout = true; rej(e); }, timeoutMs) );costlens/server/analysis/cost-modeler.js (1)
150-174: No backoff between retries inrequestJsonWithRetry.The retry loop immediately re-invokes the OpenAI API on failure. If the failure is a rate-limit (429) or transient server error, instant retries are likely to fail again and burn through the retry budget. Add a short exponential delay:
Proposed fix
for (let attempt = 0; attempt <= retries; attempt++) { try { const response = await this.openai.chat.completions.create({ ... }); ... return JSON.parse(content); } catch (error) { lastError = error; + if (attempt < retries) { + await new Promise((r) => setTimeout(r, 1000 * 2 ** attempt)); + } } }
1 participant
CostLens is an open-source intelligence tool that reverse-engineers the true cost of any SaaS product by analyzing it across five pillars: Infrastructure, Build, Buyer, Risk, and Competitors.
Deployed Link :-naked-saa-s-tiny-fish.vercel.app
Summary by CodeRabbit
New Features