Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaced an imperative lazy fetch with a declarative RTK Query ( Changes
Sequence Diagram(s)sequenceDiagram
participant Sheet as LogDetailsSheet
participant Hook as useGetLogByIdQuery
participant API as ServerAPI
Sheet->>Hook: mount/open with skip: !open || !log?.id and pollingInterval
Hook->>API: GET /logs/:id (polled per pollingInterval)
API-->>Hook: 200 / log data (status: processing|complete) or error
Hook-->>Sheet: isLoading / isError / data updates
alt isError or data.status == "processing"
Sheet->>Hook: set pollingInterval = 2000
else
Sheet->>Hook: set pollingInterval = 0
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
3fc6bc1 to
101b7c9
Compare
Confidence Score: 4/5Safe to merge for typical usage, but two reliability issues from prior review threads remain unaddressed and could cause unnecessary server load in edge cases. The two issues highlighted in previous review threads — infinite polling on persistent errors and stale pollingInterval on log navigation — are still present in the code. Both are P1-level reliability concerns: a 404 or server error will cause indefinite 2-second polling for the lifetime of the open sheet, and navigating from an errored log to a new one triggers immediate polling before the new log has a chance to fail. These don't affect the happy path but are real defects on the changed code paths. ui/app/workspace/logs/sheets/logDetailsSheet.tsx — polling lifecycle management (lines 88–96) Important Files Changed
Reviews (4): Last reviewed commit: "Fix: Added poll to the log details sheet..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/logs/sheets/logDetailsSheet.tsx`:
- Around line 88-95: Polling is currently only enabled on errors; change the
logic so pollingInterval is set when the log is in a "processing" state or when
isError is true. Update the effect that calls setPollingInterval to check
log?.status === PROCESSING (import the PROCESSING constant from
ui/lib/constants/logs.ts) || isError and set pollingInterval (e.g. 2000)
accordingly. Also stop using !isFetching as the readiness gate during refetches
— rely on the presence of fullLog or the query's isLoading flag for initial load
and avoid gating rendering on isFetching to prevent UI flicker during background
polling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b6346a3-61bd-45ce-b045-280b879a41b0
📒 Files selected for processing (2)
ui/app/workspace/logs/sheets/logDetailsSheet.tsxui/lib/store/apis/logsApi.ts
101b7c9 to
5e124a7
Compare
ff789b8 to
fd6ed11
Compare
5e124a7 to
0f7d254
Compare
Merge activity
|
The base branch was changed.
0f7d254 to
28371c8
Compare
* v1.4.17-release-cut (#2368) * fixed helm schema fix (#2369) ## Summary Remove Bullfrog security monitoring from GitHub Actions workflows and update Helm chart schema validation requirements. ## Changes - Removed `bullfrogsec/bullfrog@7bc9b6e13e2dd9cbe5861f33bc26dc6bdb9d9ed2` action with `egress-policy: audit` from all GitHub Actions workflows - Updated Helm chart values schema to only require `dimension` field instead of `dimension`, `keys`, and `provider` for config objects ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Verify that GitHub Actions workflows execute successfully without the Bullfrog security step: ```sh # Trigger any workflow to ensure it runs without errors # Check that Helm chart validation accepts configs with only dimension field helm lint helm-charts/bifrost/ ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations This change removes network egress monitoring from CI/CD pipelines. Ensure alternative security measures are in place if network monitoring is still required. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * fix migration tests (#2371) * framework: bump core to v1.4.15 --skip-pipeline * plugins/governance: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/jsonparser: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/litellmcompat: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/logging: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/maxim: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/mocker: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/otel: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/semanticcache: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * plugins/telemetry: bump core to v1.4.15 and framework to v1.2.34 --skip-pipeline * test fixes (#2374) ## Summary Adds support for testing streaming multiple tool calls functionality across LLM providers. This addresses the need to differentiate between providers that support multiple tool calls in streaming mode versus those that only return one tool call at a time during streaming. ## Changes - Added `MultipleToolCallsStreaming` field to `TestScenarios` struct to track streaming multiple tool calls capability - Updated all provider test configurations to enable the new streaming multiple tool calls feature - Added conditional test skipping in multiple tool calls streaming tests when providers don't support this functionality - Refined streaming response validation expectations to handle consolidated responses from chunks more accurately - Updated provider-specific validation expectations for Cohere and Parasail to reflect their actual response formats ## Type of change - [x] Feature - [x] Refactor ## Affected areas - [x] Core (Go) - [x] Providers/Integrations ## How to test Run the LLM provider tests to validate streaming multiple tool calls functionality: ```sh # Core/Transports go version go test ./... # Test specific provider streaming multiple tool calls go test ./core/providers/openai -v -run TestOpenAI go test ./core/providers/anthropic -v -run TestAnthropic ``` The tests will automatically skip streaming multiple tool calls scenarios for providers that don't support this feature. ## Screenshots/Recordings N/A - Backend testing enhancement only. ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations No security implications - this is a testing infrastructure enhancement. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * transports: update dependencies --skip-pipeline * Adds changelog for v1.4.18 --skip-pipeline * [StepSecurity] Apply security best practices (#2372) * [StepSecurity] Apply security best practices Signed-off-by: StepSecurity Bot <bot@stepsecurity.io> * Update .github/workflows/codeql.yml Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * cr fixes * fixed cr comments 2 --------- Signed-off-by: StepSecurity Bot <bot@stepsecurity.io> Co-authored-by: Akshay Deo <akshay@akshaydeo.com> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * removed codeql to fallback on default setup (#2376) ## Summary Removes the CodeQL security analysis workflow from the GitHub Actions configuration. This eliminates automated static code analysis for Go, JavaScript, and Python languages that was previously running on pushes, pull requests, and weekly schedules. ## Changes - Deleted `.github/workflows/codeql.yml` which contained the complete CodeQL workflow configuration - Removed automated security scanning for Go, JavaScript, and Python codebases - Eliminated the scheduled weekly security analysis runs ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Verify that CodeQL workflow is no longer present in the repository: ```sh # Confirm the workflow file has been removed ls -la .github/workflows/ # Should not show codeql.yml # Check GitHub Actions tab to ensure CodeQL runs are no longer scheduled # Navigate to repository Actions tab and verify no CodeQL workflows appear ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations This change removes automated security analysis from the CI/CD pipeline. The repository will no longer benefit from CodeQL's static analysis capabilities for detecting security vulnerabilities, code quality issues, and potential bugs in Go, JavaScript, and Python code. Consider alternative security scanning solutions if this workflow was providing valuable security insights. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * Update snyk.yml (#2380) * fips base image (#2310) ## Summary Updates the transports Docker configuration to use a FIPS-compliant base image and removes the built-in health check mechanism. ## Changes - Replaced `alpine:3.23.3` base image with `dhi.io/alpine-base:3.23-alpine3.23-fips` for FIPS compliance - Removed the Docker HEALTHCHECK directive that was monitoring the `/health` endpoint ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Verify the Docker image builds successfully with the new base image and that the application starts correctly without the health check. ```sh # Build the Docker image docker build -t transports-test ./transports # Run the container docker run -p 8080:8080 transports-test # Verify the application is running curl http://localhost:8080/health ``` ## Screenshots/Recordings N/A ## Breaking changes - [x] Yes - [ ] No The removal of the Docker HEALTHCHECK may affect container orchestration systems that rely on Docker's built-in health checking. External health monitoring will need to be configured if required. ## Related issues N/A ## Security considerations This change enhances security by adopting a FIPS-compliant base image, which provides cryptographic modules that meet Federal Information Processing Standards. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable * fixed snyk failure and enterprise update (#2382) ## Summary Enhances the model catalog to extract and cache max output tokens from pricing data, improves Snyk workflow reliability by checking for SARIF file existence before upload, and updates security documentation to reflect full SHA pinning for the enterprise repository. ## Changes - Added `MaxOutputTokens` field to `PricingEntry` struct to capture model parameter limits from pricing datasheet - Implemented `populateModelParamsFromPricing()` function to extract max output tokens and populate the model params cache - Refactored model name extraction logic into reusable `extractModelName()` utility function - Enhanced Snyk workflow conditions to only upload SARIF files when they exist, preventing upload failures - Updated security documentation to reflect that bifrost-enterprise now uses full SHA pinning (100% coverage) ## Type of change - [x] Feature - [x] Chore/CI - [x] Documentation ## Affected areas - [x] Core (Go) - [x] Docs ## How to test Verify model catalog functionality and pricing data processing: ```sh # Core functionality go version go test ./framework/modelcatalog/... # Test pricing sync with max_output_tokens go test -v ./framework/modelcatalog/ -run TestSyncPricing # Verify Snyk workflow changes in CI # Check that SARIF upload steps only run when files exist ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations - Improves CI security by preventing potential failures in SARIF upload steps - Documents enhanced security posture with full SHA pinning in enterprise repository ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * fix snyk failures (#2385) ## Summary Improves Docker container security and reliability by migrating from custom base image to standard Alpine, adding health checks, and enhancing Snyk security scanning. ## Changes - **Docker base image migration**: Replaced `bifrosthq/dhi-alpine-base:3.22-fips_bifrost-v27032026` with standard `alpine:3.23.3` for better security and maintainability - **Added health checks**: Implemented HTTP health check endpoint monitoring with 30s intervals and proper retry logic - **Enhanced user management**: Consolidated user creation and permission setup into single RUN command for better layer optimization - **Improved Snyk scanning**: Added build step before security scanning and excluded `examples` and `tests/scripts` directories from vulnerability analysis - **Runtime dependencies**: Explicitly installed required CGO runtime libraries (musl, libgcc, ca-certificates, wget) ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Validate Docker builds and container health: ```sh # Build and test standard Dockerfile docker build -f transports/Dockerfile -t bifrost:test . docker run -d --name bifrost-test -p 8080:8080 bifrost:test docker ps # Should show healthy status after ~35s curl http://localhost:8080/health # Should return 200 OK # Build and test local development Dockerfile docker build -f transports/Dockerfile.local -t bifrost:local . docker run -d --name bifrost-local -p 8081:8080 bifrost:local docker ps # Should show healthy status after ~35s # Test Snyk workflow make build # Verify build step works # Run Snyk scan (requires SNYK_TOKEN) ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations - Migrated from custom base image to well-maintained Alpine Linux for better security patching - Enhanced Snyk scanning excludes test directories to focus on production code vulnerabilities - Health check endpoint provides better container monitoring capabilities - Explicit runtime dependency management reduces attack surface ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * fix: add explicit type="button" to navigation and action buttons (#2353) ## Summary Added explicit `type="button"` attributes to Button components in log detail sheets and plugin sequence forms to prevent unintended form submissions when these buttons are clicked. ## Changes - Added `type="button"` to navigation buttons (Previous/Next) in log detail sheets - Added `type="button"` to dropdown menu trigger buttons in log detail sheets - Added `type="button"` to the Save Sequence button in plugin sequence sheet - Added `type="button"` to the Copy button in CEL rule builder This prevents these buttons from accidentally triggering form submissions when used within forms, ensuring they only perform their intended click actions. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (Next.js) - [ ] Docs ## How to test Verify that buttons in log detail sheets and plugin forms work correctly without triggering form submissions: 1. Open log detail sheets and test navigation buttons 2. Test dropdown menu triggers in log sheets 3. Test plugin sequence save functionality 4. Test CEL rule builder copy button ```sh # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings N/A - This is a behavioral fix without visual changes. ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations None - this change only affects button behavior to prevent unintended form submissions. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * fix: prevent edit mode activation when clicking interactive elements in message views (#2377) ## Summary Prevents edit mode from being triggered when clicking on interactive elements (buttons, links, or elements with button role) within message views. This fixes the issue where clicking on buttons or links inside messages would unintentionally activate edit mode. ## Changes - Added event target checking in onClick handlers for assistant, system, and user message views - Modified click handlers to check if the clicked element is within a button, link, or element with button role using `closest()` method - Restructured conditional logic for better readability and early returns ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (Next.js) - [ ] Docs ## How to test 1. Navigate to a message view with interactive elements (buttons, links) 2. Click on buttons or links within the message content 3. Verify that edit mode is not triggered when clicking interactive elements 4. Click on non-interactive areas of the message to confirm edit mode still works 5. Test across all message types (assistant, system, user) ```sh # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations No security implications - this is a UI interaction improvement. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * refactor: extract copy-to-clipboard logic into reusable hook (#2379) ## Summary Refactored clipboard copy functionality across the UI by creating a reusable `useCopyToClipboard` hook to replace duplicate clipboard handling code. ## Changes - Created a new `useCopyToClipboard` hook that provides consistent clipboard functionality with customizable success/error messages and automatic reset of copied state - Replaced inline clipboard handling code across 11 components with the new hook - Removed direct `navigator.clipboard.writeText()` calls and manual toast notifications - Added support for customizable messages and reset delays in the hook ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (Next.js) - [ ] Docs ## How to test Test clipboard functionality across the affected components: ```sh # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` 1. Navigate to API Keys page and test copying curl examples 2. Open log details and test copying request IDs and request bodies 3. Test copy functionality in routing rules CEL builder 4. Test copy functionality in virtual keys table 5. Test copy functionality in Prometheus configuration 6. Verify all copy actions show appropriate toast messages ## Screenshots/Recordings No visual changes - functionality remains the same with improved code organization. ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations No security implications - maintains existing clipboard functionality without changes to security model. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * refactor: replace lazy query with polling query and fix overflow in log details sheet (#2381) ## Summary Improves log detail sheet reliability by switching from lazy loading to automatic polling for failed log fetches and fixes UI overflow issues in the sheet header. ## Changes - Replaced `useLazyGetLogByIdQuery` with `useGetLogByIdQuery` for automatic data fetching when the sheet opens - Added polling mechanism that retries every 2 seconds when log fetch fails, stopping when successful - Fixed horizontal overflow issues in the sheet header by adding `overflow-x-hidden` classes - Exported `useGetLogByIdQuery` hook from the logs API ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (Next.js) - [ ] Docs ## How to test 1. Open the logs page and click on a log entry to view details 2. Verify the log details load automatically without manual triggering 3. Test with network issues or slow responses to confirm polling retry behavior 4. Check that long request IDs don't cause horizontal overflow in the sheet header ```sh # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations None - this change only affects UI data fetching patterns and styling. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable * merge-branch-main-into-v-1-5-0 resolve * merge-branch-main-into-v-1-5-0 resolve --------- Signed-off-by: StepSecurity Bot <bot@stepsecurity.io> Co-authored-by: Akshay Deo <akshay@akshaydeo.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: StepSecurity Bot <bot@stepsecurity.io> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

Summary
Improves log detail sheet reliability by switching from lazy loading to automatic polling for failed log fetches and fixes UI overflow issues in the sheet header.
Changes
useLazyGetLogByIdQuerywithuseGetLogByIdQueryfor automatic data fetching when the sheet opensoverflow-x-hiddenclassesuseGetLogByIdQueryhook from the logs APIType of change
Affected areas
How to test
Screenshots/Recordings
N/A
Breaking changes
Related issues
N/A
Security considerations
None - this change only affects UI data fetching patterns and styling.
Checklist
docs/contributing/README.mdand followed the guidelines