-
-
Notifications
You must be signed in to change notification settings - Fork 603
chore(internals): remove python script result data type debug log #4807
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
chore(internals): remove python script result data type debug log #4807
Conversation
⏩ No test execution environment matched (1cbb29d) View output ↗ Tip New to Tusk? Learn more here. |
📝 WalkthroughWalkthroughThe changes remove a block of debug logging from the Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/python/pythonUtils.ts (2)
195-201
: Stale blank line left after log-statement removalThe noisy debug line was successfully dropped (👍), but an empty line was left behind, leaving two consecutive blank lines (198–200). Trivial, yet trimming it keeps the file tidy and avoids clutter in future diffs.
151-155
: Synchronous FS write blocks the event-loop &await
is a no-op
fs.writeFileSync
is synchronous, soawait
does nothing and the call blocks the node process until the write completes. Unless the script absolutely relies on this blocking behaviour, prefer the promise-based variant to keep the I/O non-blocking:- await fs.writeFileSync(tempJsonPath, safeJsonStringify(args) as string, 'utf-8'); + await fs.promises.writeFile(tempJsonPath, safeJsonStringify(args) as string, 'utf-8');Same applies to the
readFileSync
(line 179) andunlinkSync
calls in the cleanup block.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/python/pythonUtils.ts
(1 hunks)test/python/pythonUtils.test.ts
(0 hunks)
💤 Files with no reviewable changes (1)
- test/python/pythonUtils.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
`**/*.{ts,tsx}`: Use TypeScript with strict type checking.
**/*.{ts,tsx}
: Use TypeScript with strict type checking.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
src/python/pythonUtils.ts
`**/*.{js,jsx,ts,tsx}`: Follow established import order with @trivago/prettier-p...
**/*.{js,jsx,ts,tsx}
: Follow established import order with @trivago/prettier-plugin-sort-imports.
Use consistent curly braces for all control statements.
Prefer const over let; avoid var.
Use object shorthand syntax whenever possible.
Use async/await for asynchronous code.
Use consistent error handling with proper type checks.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
src/python/pythonUtils.ts
`**/*.{ts,tsx}`: Prefer not to introduce new TypeScript types; use existing interfaces whenever possible
**/*.{ts,tsx}
: Prefer not to introduce new TypeScript types; use existing interfaces whenever possible
📄 Source: CodeRabbit Inference Engine (.cursor/rules/gh-cli-workflow.mdc)
List of files the instruction was applied to:
src/python/pythonUtils.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: promptfoo/promptfoo#0
File: .cursor/rules/gh-cli-workflow.mdc:0-0
Timestamp: 2025-06-30T13:44:03.663Z
Learning: Avoid large, unrelated changes in a single PR
Learnt from: CR
PR: promptfoo/promptfoo#0
File: .cursor/rules/python.mdc:0-0
Timestamp: 2025-06-23T12:49:38.363Z
Learning: When integrating Python components with the TypeScript core of the promptfoo project, follow the patterns established in existing Python files for seamless interfacing.
Learnt from: CR
PR: promptfoo/promptfoo#0
File: .cursor/rules/gh-cli-workflow.mdc:0-0
Timestamp: 2025-06-30T13:44:03.663Z
Learning: Applies to **/*.{ts,tsx} : Prefer not to introduce new TypeScript types; use existing interfaces whenever possible
Learnt from: CR
PR: promptfoo/promptfoo#0
File: .cursor/rules/gh-cli-workflow.mdc:0-0
Timestamp: 2025-06-30T13:44:03.663Z
Learning: Applies to **/*.{test,spec}.{js,ts,tsx} : Avoid disabling or skipping tests unless absolutely necessary and documented
Learnt from: CR
PR: promptfoo/promptfoo#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T13:43:03.656Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use consistent error handling with proper type checks.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test on Node 18.x and macOS-latest
- GitHub Check: Test on Node 18.x and windows-latest
- GitHub Check: Redteam Custom Enterprise Server
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.
Looks good after linter passes
Removes debug logging of Python script return data type. In practice, I've found it to be noise that makes the logs more difficult to read, for example: