-
Notifications
You must be signed in to change notification settings - Fork 36
Fix Database Path in DailyRefinementJob #574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,14 +25,14 @@ | |
| "devDependencies": { | ||
| "@babel/preset-env": "^7.29.2", | ||
| "@babel/preset-react": "^7.28.5", | ||
| "@types/jest": "^29.5.12", | ||
| "@types/jest": "^29.5.14", | ||
| "@types/node": "^20.11.24", | ||
| "@types/node-cron": "^3.0.11", | ||
| "@types/sqlite3": "^3.1.11", | ||
| "jest": "^29.7.0", | ||
| "jest-environment-jsdom": "^30.3.0", | ||
| "jest-util": "^30.2.0", | ||
| "ts-jest": "^29.1.2", | ||
| "jest-util": "^30.3.0", | ||
| "ts-jest": "^29.4.6", | ||
|
||
| "ts-node": "^10.9.2", | ||
| "typescript": "^5.3.3" | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -8,7 +8,7 @@ import { Issue } from "../services/types"; | |||||
|
|
||||||
| // Load environmental or fallback to test.db or production db | ||||||
|
||||||
| // Load environmental or fallback to test.db or production db | |
| // Use DB_PATH env var if set, otherwise fall back to ../data/issues.db |
Copilot
AI
Mar 22, 2026
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.
This change fixes the default SQLite path, but there’s no test that would fail if this regressed (e.g., asserting what path is used when DB_PATH is unset). Consider making the path derivation injectable/exported and adding a focused Jest test so the intended default (data/issues.db) is enforced.
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.
Create the fallback directory before opening the SQLite file.
This path fix is good, but it can still fail on fresh environments if ../data doesn’t exist. Ensure the parent directory exists before new sqlite3.Database(...) is called.
Suggested patch
import * as cron from "node-cron";
import * as sqlite3 from "sqlite3";
import * as path from "path";
+import * as fs from "fs";
import { TrendAnalyzer } from "../services/trendAnalyzer";
import { AdaptiveWeights } from "../services/adaptiveWeights";
import { IntelligenceIndex } from "../services/intelligenceIndex";
import { Issue } from "../services/types";
// Load environmental or fallback to test.db or production db
const dbPath =
process.env.DB_PATH || path.join(__dirname, "../data/issues.db");
+fs.mkdirSync(path.dirname(dbPath), { recursive: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scheduler/dailyRefinementJob.ts` at line 11, The code computes the DB path
using process.env.DB_PATH || path.join(__dirname, "../data/issues.db") but may
attempt to open the SQLite file before its parent directory exists; before
calling new sqlite3.Database(...) ensure the directory for that computed dbPath
exists (use path.dirname(dbPath) and create it with recursive mkdir), then
proceed to open the database so new sqlite3.Database(...) won't fail on fresh
environments.
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.
Jest-related devDependencies are now a mix of Jest v29 (
jest) and Jest v30 packages (jest-environment-jsdom,jest-util). This pulls Jest 30 internals into the lockfile and can cause subtle resolver/version skew. Prefer aligning all Jest packages to the same major version (or removingjest-util/jest-environment-jsdomif they aren’t needed for this repo’s Jest config).