fix(audit): System Audit Remediation and Improvements#1262
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,36 @@ | ||||||||||||||||||||||||||
| export interface RAGPreflightChecks { | ||||||||||||||||||||||||||
| maxTokens?: number; | ||||||||||||||||||||||||||
| maxCost?: number; | ||||||||||||||||||||||||||
| requiredQualityThreshold?: number; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| export function validatePreflightConstraints( | ||||||||||||||||||||||||||
| estimatedTokens: number, | ||||||||||||||||||||||||||
| estimatedCost: number, | ||||||||||||||||||||||||||
| constraints: RAGPreflightChecks, | ||||||||||||||||||||||||||
| ): string[] { | ||||||||||||||||||||||||||
| const errors: string[] = []; | ||||||||||||||||||||||||||
| if (constraints.maxTokens && estimatedTokens > constraints.maxTokens) { | ||||||||||||||||||||||||||
| errors.push(`Estimated tokens (${estimatedTokens}) exceeds maximum allowed (${constraints.maxTokens}).`); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if (constraints.maxCost && estimatedCost > constraints.maxCost) { | ||||||||||||||||||||||||||
| errors.push(`Estimated cost ($${estimatedCost}) exceeds maximum allowed ($${constraints.maxCost}).`); | ||||||||||||||||||||||||||
Comment on lines
+14
to
+19
Contributor There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Do not skip zero-valued limits. The checks at Line 14 and Line 18 are bypassed when Proposed fix- if (constraints.maxTokens && estimatedTokens > constraints.maxTokens) {+ if (constraints.maxTokens !== undefined && estimatedTokens > constraints.maxTokens) {
...
- if (constraints.maxCost && estimatedCost > constraints.maxCost) {+ if (constraints.maxCost !== undefined && estimatedCost > constraints.maxCost) {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| return errors; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| export function getOfflineEnvironment() { | ||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||
| RAG_PROVIDER_MODE: "offline", | ||||||||||||||||||||||||||
| OPENAI_API_KEY: "", | ||||||||||||||||||||||||||
| OPENAI_ORG_ID: "", | ||||||||||||||||||||||||||
| OPENAI_PROJECT_ID: "", | ||||||||||||||||||||||||||
| NEXT_PUBLIC_SUPABASE_URL: "https://offline.invalid", | ||||||||||||||||||||||||||
| NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY: "offline-placeholder", | ||||||||||||||||||||||||||
| SUPABASE_SERVICE_ROLE_KEY: "offline-placeholder", | ||||||||||||||||||||||||||
| SUPABASE_DB_URL: "postgresql://offline:offline@offline.invalid:5432/offline", | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| import { spawn } from "node:child_process"; | ||
| const parallelTasks = [ | ||
| "check:runtime", | ||
| "check:github-actions", | ||
| "check:ci-scope", | ||
| "check:ci-triage", | ||
| "check:pr-policy", | ||
| "check:gate-manifest", | ||
| "check:branch-review-ledger", | ||
| "sitemap:check", | ||
| "docs:check-index", | ||
| "docs:check-scripts", | ||
| "docs:check-links", | ||
| "check:knip", | ||
| "check:maintainability-budgets", | ||
| "brand:check", | ||
| "check:assets", | ||
| "check:therapy-data-index", | ||
| "check:type-scale", | ||
| "check:icon-scale", | ||
| "check:design-system-contract", | ||
| "check:migration-role", | ||
| "check:function-grants", | ||
| "check:owner-scope", | ||
| "lint", | ||
| ]; | ||
| async function runTask(task) { | ||
| return new Promise((resolve, reject) => { | ||
| const child = spawn("npm", ["run", task], { shell: true, stdio: "pipe" }); | ||
| let output = ""; | ||
| child.stdout.on("data", (data) => (output += data.toString())); | ||
| child.stderr.on("data", (data) => (output += data.toString())); | ||
| child.on("close", (code) => { | ||
| if (code !== 0) { | ||
| console.error(output); | ||
| reject(new Error(`${task}`)); | ||
| } else { | ||
| console.log(`✔ ${task}`); | ||
| resolve(); | ||
| } | ||
| }); | ||
| }); | ||
| } | ||
| async function run() { | ||
| console.log("Starting parallel verification..."); | ||
| try { | ||
| await Promise.all(parallelTasks.map(runTask)); | ||
There was a problem hiding this comment.
When AGENTS.md reference: AGENTS.md:L167-L168 Useful? React with 👍 / 👎. | ||
| await runTask("typecheck"); | ||
| console.log("Starting tests..."); | ||
| await new Promise((resolve, reject) => { | ||
| const child = spawn("npm", ["run", "test"], { shell: true, stdio: "inherit" }); | ||
| child.on("close", (code) => { | ||
| if (code !== 0) reject(new Error("test")); | ||
| else resolve(); | ||
| }); | ||
| }); | ||
| } catch (err) { | ||
| console.error(`\n✘ [FAILED] ${err.message}`); | ||
| process.exit(1); | ||
There was a problem hiding this comment.
When any fast gate fails while slower gates are still running, Useful? React with 👍 / 👎.
Comment on lines
+50
to
+63
Contributor There was a problem hiding this comment. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bashset -euo pipefail
echo"## Locate file"
git ls-files | rg '(^|/)verify-cheap-parallel\.mjs$'||trueecho"## File excerpt"if [ -f scripts/verify-cheap-parallel.mjs ];then
wc -l scripts/verify-cheap-parallel.mjs
sed -n '1,140p' scripts/verify-cheap-parallel.mjs | cat -n
fiecho"## Search for cleanup/cancellation patterns"
rg -n "Promise\.all|Promise\.allSettled|spawn\(|child\.(unref|kill|stdin|stdout|stderr)|on\('close'|on\(\"close\"|process\.exit" scripts/verify-cheap-parallel.mjs ||trueRepository: BigSimmo/Database Length of output: 2940 🌐 Web query:
💡 Result: In Node.js, calling process.exit in the parent process does not automatically terminate or kill forked child processes [1][2]. When the parent process terminates, its child processes become "orphaned" (unless they were spawned as detached processes, in which case they continue running independently) [3][1]. Key points regarding the behavior: 1. Parent Termination: If the parent process exits, forked child processes continue to run unless explicitly handled [1][2]. Node.js does not automatically manage the lifecycle of children once the parent terminates [2]. 2. Manual Cleanup: To ensure child processes are terminated when the parent exits, you must explicitly manage this in your code [2]. A common approach is to listen for signals (such as SIGTERM or SIGINT) or the 'exit' event in the parent process, and then call subprocess.kill on each active child process [4][2]. 3. Child Self-Termination: A child process can exit itself by calling process.exit [5][4][6]. This is often the cleanest way to shut down a child process when its work is complete [4]. 4. IPC Channels: If a child process is forked with an IPC channel, the child can potentially detect when the parent process disconnects (via the 'disconnect' event) and shut itself down gracefully [5][7]. 5. Detached Processes: If you spawn a process with the detached: true option, it is intended to run independently of the parent process, and the parent will not automatically terminate it upon exiting [3][1]. If you notice child processes hanging after a parent process exits, it is likely because they were not explicitly signaled to shut down [4][1]. Implementing a cleanup mechanism in the parent's exit handler is the standard practice for ensuring all child processes are terminated [4][2]. Citations:
Terminate remaining verification tasks when one task fails.
🤖 Prompt for AI Agents | ||
| } | ||
| } | ||
| run(); | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Playwright testConfig expect toHaveScreenshot toMatchSnapshot maxDiffPixelRatio option💡 Result:
In Playwright, the maxDiffPixelRatio option is used to define an acceptable ratio of differing pixels (between 0 and 1) compared to the total number of pixels in an image during snapshot comparisons [1][2]. Both toHaveScreenshot and toMatchSnapshot support this option [1][2]. You can configure maxDiffPixelRatio globally or per project in your Playwright test configuration, as well as override it on a per-test basis [3]. Global Configuration To set a default value for all tests, use the defineConfig function in your playwright.config.ts file [3]: import { defineConfig } from '
@playwright/test'; export default defineConfig({ expect: { toHaveScreenshot: { maxDiffPixelRatio: 0.1, // Example: 10% tolerance }, toMatchSnapshot: { maxDiffPixelRatio: 0.1, }, }, }); Usage in Tests You can also pass the option directly within your test assertions to override the global setting [3][2]: // Using toHaveScreenshot await expect(page).toHaveScreenshot('example.png', { maxDiffPixelRatio: 0.05, }); // Using toMatchSnapshot expect(await page.screenshot).toMatchSnapshot('example.png', { maxDiffPixelRatio: 0.05, }); Key Differences While both methods support pixel comparison options like maxDiffPixelRatio and maxDiffPixels, they are intended for different use cases [3][4]: toHaveScreenshot: Designed specifically for visual regression testing of full pages or locators [1][3]. It includes built-in features for waiting for screenshots to stabilize and handles image comparison automatically [1]. toMatchSnapshot: A more general-purpose matcher that compares arbitrary data, including text, binary data, and images [3][2]. It does not include the specialized screenshot stabilization features found in toHaveScreenshot [1][2].Citations:
toHaveScreenshotandtoMatchSnapshotmicrosoft/playwright#21632🏁 Script executed:
Repository: BigSimmo/Database
Length of output: 50374
🏁 Script executed:
Repository: BigSimmo/Database
Length of output: 16570
Tighten the visual diff guard for
@layervisual changesmaxDiffPixelRatio: 0.05is accepted by Playwright 1.61.1 for both matchers, but it permits roughly 46k differing pixels at 1280×720. Since this PR changes the CSS cascade, use a tighter global ratio unless there’s noise that truly requires it, and keep mobile/!importantstyling covered by lower-tolerance visual checks.🤖 Prompt for AI Agents