Skip to content

fix: preserve large numeric IDs in search params by skipping JSON parse for plain strings - #3692

Merged
akshaydeo merged 1 commit into
mainfrom
05-22-fix_preserve_large_number_precision_in_query_params_for_filters
May 22, 2026
Merged

fix: preserve large numeric IDs in search params by skipping JSON parse for plain strings#3692
akshaydeo merged 1 commit into
mainfrom
05-22-fix_preserve_large_number_precision_in_query_params_for_filters

Conversation

@impoiler

@impoiler impoiler commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Prevents large numeric IDs (e.g., 64-bit integers) from losing precision when parsed from URL search parameters. Previously, all search values were passed through JSON.parse, which coerces large numbers to JavaScript's Number type and silently truncates them. This fix ensures only structured JSON values (objects and arrays) are parsed, while plain strings and numbers are left as-is.

Changes

  • Introduced a safeJsonParse function that only calls JSON.parse on values beginning with { or [, leaving all other values as raw strings to avoid numeric precision loss.
  • Configured the router to use parseSearchWith(safeJsonParse) instead of the default search parser.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

Navigate to a route that includes a large numeric ID (greater than Number.MAX_SAFE_INTEGER) in the URL search parameters and verify the ID is preserved exactly without truncation.

cd ui
pnpm i || npm i
pnpm build || npm run build

Screenshots/Recordings

N/A

Breaking changes

  • Yes
  • No

Related issues

N/A

Security considerations

No security implications. This change only affects client-side URL search parameter parsing.

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

@coderabbitai

coderabbitai Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c9913ad0-829e-4716-8f39-8810f1b5b16c

📥 Commits

Reviewing files that changed from the base of the PR and between b19776f and c63a749.

📒 Files selected for processing (2)
  • framework/logstore/matviews.go
  • ui/app/main.tsx

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Router search handling now safely parses structured JSON (objects/arrays) while preserving non-JSON primitives, reducing parsing errors and keeping URL params stable.
    • User filter display now falls back to user ID when a user name is missing or empty, ensuring consistent display and filtering.

Walkthrough

Adds a guarded safeJsonParse and configures TanStack Router with parseSearchWith(safeJsonParse) to only JSON-parse objects/arrays from the URL search; updates mv_filter_users to set name to COALESCE(NULLIF(user_name, ''), user_id).

Changes

Search parameter parsing safeguard

Layer / File(s) Summary
Safe JSON parsing for search parameters
ui/app/main.tsx
Adds safeJsonParse which JSON-parses only values starting with { or [ and wires it into router initialization via parseSearch: parseSearchWith(safeJsonParse), preserving primitive search values.

Materialized view updates

Layer / File(s) Summary
Use COALESCE(NULLIF(user_name, ''), user_id) for user display name
framework/logstore/matviews.go
mv_filter_users now sets name to COALESCE(NULLIF(user_name, ''), user_id) so empty or null user_name falls back to user_id in the view.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • akshaydeo

Poem

I nibble through query strings with care,
Braces, brackets — I only parse what's there,
Plain words I leave on their merry way,
While names that vanish get IDs today,
Hops and bytes, a rabbit's tidy care 🐰

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: fixing numeric ID precision loss by avoiding JSON parsing on plain strings in search params.
Description check ✅ Passed The description covers all critical sections: it explains the problem, details the solution, specifies the change type and affected areas, provides testing instructions, and addresses breaking changes and security considerations.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 05-22-fix_preserve_large_number_precision_in_query_params_for_filters

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.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Comment @coderabbitai help to get the list of available commands and usage tips.

impoiler commented May 22, 2026

Copy link
Copy Markdown
Contributor Author

@impoiler impoiler changed the title fix: preserve large number precision in query params for filters fix: preserve large numeric IDs in search params by skipping JSON parse for plain strings May 22, 2026
@impoiler impoiler self-assigned this May 22, 2026
@impoiler
impoiler marked this pull request as ready for review May 22, 2026 10:16
@coderabbitai
coderabbitai Bot requested a review from akshaydeo May 22, 2026 10:16
@akshaydeo
akshaydeo changed the base branch from 05-22-fix_show_user_name_for_filters_dropdown_in_logs to graphite-base/3692 May 22, 2026 10:17
@akshaydeo
akshaydeo force-pushed the graphite-base/3692 branch from eee42c2 to da4ffe4 Compare May 22, 2026 10:18
@akshaydeo
akshaydeo force-pushed the 05-22-fix_preserve_large_number_precision_in_query_params_for_filters branch from 3758a45 to 9393d9e Compare May 22, 2026 10:18
@graphite-app
graphite-app Bot changed the base branch from graphite-base/3692 to main May 22, 2026 10:18
@akshaydeo
akshaydeo force-pushed the 05-22-fix_preserve_large_number_precision_in_query_params_for_filters branch from 9393d9e to 9b28643 Compare May 22, 2026 10:18

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ui/app/main.tsx`:
- Around line 12-18: safeJsonParse currently calls JSON.parse for values
starting with "{" or "[" which will throw on malformed JSON and can break route
parsing via parseSearchWith(safeJsonParse); wrap the JSON.parse call in a
try/catch inside safeJsonParse, return the original value (the raw string) on
parse errors, and keep successful parsed objects/arrays returned as before so
navigation doesn't crash on bad URLs.
🪄 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: 192e34e1-2e42-44f7-b6a8-47739edae912

📥 Commits

Reviewing files that changed from the base of the PR and between eee42c2 and 3758a45.

📒 Files selected for processing (1)
  • ui/app/main.tsx

Comment thread ui/app/main.tsx
@impoiler
impoiler force-pushed the 05-22-fix_preserve_large_number_precision_in_query_params_for_filters branch from 9b28643 to f2e2c91 Compare May 22, 2026 10:20
coderabbitai[bot]
coderabbitai Bot previously approved these changes May 22, 2026
coderabbitai[bot]
coderabbitai Bot previously approved these changes May 22, 2026
@greptile-apps

greptile-apps Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

Safe to merge the UI change; the Go matview fix needs a mechanism to rebuild the existing view on upgrade.

The ui/app/main.tsx change is low-risk and self-contained. The mv_filter_users DDL change is the right fix logically, but CREATE MATERIALIZED VIEW IF NOT EXISTS means the updated COALESCE expression is never applied to an already-existing view — existing users won't see the behaviour change until the view is rebuilt manually or a drop mechanism is added.

framework/logstore/matviews.go — the selectExpr change needs a way to force-rebuild the existing mv_filter_users view on upgrade.

Important Files Changed

Filename Overview
framework/logstore/matviews.go Changes mv_filter_users selectExpr to COALESCE(NULLIF(user_name,''),user_id) AS name, but CREATE MATERIALIZED VIEW IF NOT EXISTS prevents the new DDL from being applied on existing deployments.
ui/app/main.tsx Adds safeJsonParse to skip JSON.parse for non-object/array search strings, preventing precision loss for large integer IDs passed as plain URL parameters.

Reviews (3): Last reviewed commit: "fix: preserve large number precision in ..." | Re-trigger Greptile

Comment thread ui/app/main.tsx
@impoiler
impoiler force-pushed the 05-22-fix_preserve_large_number_precision_in_query_params_for_filters branch from f2e2c91 to b19776f Compare May 22, 2026 10:42

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@framework/logstore/matviews.go`:
- Line 223: The SELECT expression uses COALESCE(user_name, user_id) which treats
empty strings as valid values; update the selectExpr so empty user_name strings
are treated as NULL before coalescing (e.g., use NULLIF or an equivalent) so the
name column falls back to user_id; locate the selectExpr string in matviews.go
and replace COALESCE(user_name, user_id) with COALESCE(NULLIF(user_name, ''),
user_id) (keeping the aliases "user_id AS id" and "AS name" unchanged).
🪄 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: f14e7403-a025-4453-8b72-efddd2b75078

📥 Commits

Reviewing files that changed from the base of the PR and between f2e2c91 and b19776f.

📒 Files selected for processing (2)
  • framework/logstore/matviews.go
  • ui/app/main.tsx

Comment thread framework/logstore/matviews.go Outdated
@impoiler
impoiler force-pushed the 05-22-fix_preserve_large_number_precision_in_query_params_for_filters branch from b19776f to c63a749 Compare May 22, 2026 11:03

akshaydeo commented May 22, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • May 22, 11:07 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 22, 11:07 AM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo merged commit 0563140 into main May 22, 2026
14 checks passed
@akshaydeo
akshaydeo deleted the 05-22-fix_preserve_large_number_precision_in_query_params_for_filters branch May 22, 2026 11:07
Vaibhav701161 pushed a commit that referenced this pull request May 26, 2026
…se for plain strings (#3692)

## Summary

Prevents large numeric IDs (e.g., 64-bit integers) from losing precision when parsed from URL search parameters. Previously, all search values were passed through `JSON.parse`, which coerces large numbers to JavaScript's `Number` type and silently truncates them. This fix ensures only structured JSON values (objects and arrays) are parsed, while plain strings and numbers are left as-is.

## Changes

- Introduced a `safeJsonParse` function that only calls `JSON.parse` on values beginning with `{` or `[`, leaving all other values as raw strings to avoid numeric precision loss.
- Configured the router to use `parseSearchWith(safeJsonParse)` instead of the default search parser.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

Navigate to a route that includes a large numeric ID (greater than `Number.MAX_SAFE_INTEGER`) in the URL search parameters and verify the ID is preserved exactly without truncation.

```sh
cd ui
pnpm i || npm i
pnpm build || npm run build
```

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

N/A

## Security considerations

No security implications. This change only affects client-side URL search parameter parsing.

## 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
@akshaydeo akshaydeo mentioned this pull request May 26, 2026
akshaydeo added a commit that referenced this pull request May 26, 2026
## ✨ Features

- **Azure v1 API Migration** — Migrated Azure provider to the v1 API:
removed the `api-version` query parameter and the
`/openai/deployments/{model}/...` URL pattern in favor of
`/openai/v1/{operation}`; the `api_version` field has been dropped from
`AzureKeyConfig` (#3661, #3756)
- **EnvVar Support for OTEL & Prometheus Configs** — `CollectorURL`,
`MetricsEndpoint`, headers, push gateway URL, and basic auth credentials
can now be sourced from environment variables (e.g.,
`env.OTEL_COLLECTOR_URL`); added a new `ConfigMarshallerPlugin`
interface that lets plugins control storage/redaction round-trips
(#3651)
- **OTel Extra Header Forwarding** — `x-bf-eh-*` extra headers forwarded
to upstream providers are now also emitted on the request span under
`gen_ai.request.extra_header.*` for end-to-end tracing (#3730)
- **OTel Semantic Conventions** — Aligned OTel attribute keys with the
OpenTelemetry GenAI spec (canonical `gen_ai.*` and new `bifrost.*`
attributes); legacy attributes are retained in parallel to avoid
breaking existing dashboards (#3732)
- **VK Quota with Provider Configs** — `GetVirtualKeyQuotaByValue` and
the `getVirtualKeyQuota` HTTP response now include `provider_configs`
with their budgets and rate limits (#3721)
- **MCP Temp Token Non-Auth Toggle** — Added
`mcp_enable_temp_token_auth` client config flag to gate short-lived MCP
token minting for non-authenticated users (#3720)
- **Responses Stream in JSON Parser** — `jsonparser` plugin now handles
OpenAI Responses API streaming (`ResponsesStreamRequest`) in addition to
chat completions (#3749)
- **Session API Rework** — Logout now calls both the password-based
session logout and OAuth logout endpoints and resets all RTK Query cache
state (#3698)

## 🐞 Fixed

- **Streaming Latency for Observability** — Deferred root span
termination to the trace completer callback for streaming requests so
request latency is no longer inflated by header-flush time (#3762)
- **Stream Cancellation Race** — Set `BifrostContextKeyConnectionClosed`
before closing the stream and short-circuit `idleTimeoutReader.Read`
when the connection is already closed to avoid panics and hangs on
cancellation (#3733)
- **Bedrock Cache Points** — Strip cache points from Bedrock requests
for models that do not support prompt caching (e.g., GLM, Llama) to
avoid Converse API errors (#3754)
- **Bedrock Empty Text Blocks** — Skip empty/nil text blocks during
Bedrock response conversion to avoid invalid messages (#3747)
- **Bedrock Reasoning + Tools** — Preserve reasoning content blocks on
assistant turns that also contain tool calls in the Bedrock chat
converter (#3690)
- **Bedrock Search Content & Video** — Restored search content and video
parts that were being dropped from Bedrock-native passthrough requests
(#3729)
- **Structured Output Stop Reason** — Fixed an incorrect `tool_calls`
finish reason when structured output is combined with extended-thinking
tools (#3685)
- **Gemini Tool Schema Passthrough** — Forward full tool parameter
schemas via `parametersJsonSchema` instead of the lossy `parameters`
form; corrected tool response role to `user`; resolved structured output
+ tools conflict (#3761)
- **Anthropic Stop Reason & Tool Versions** — Normalized stop reason
mapping (`end_turn` to `stop`, `tool_use` to `tool_calls`, `max_tokens`
to `length`) and upgraded `text_editor_20250124`/`str_replace_editor` to
`text_editor_20250728` for computer-use tools (#3761)
- **Azure Endpoint Redaction** — Fixed a panic when
`AzureKeyConfig.Endpoint` is a literal value rather than an env
reference (#3761)
- **Auth Middleware Path Match** — Match temp-token auth middleware
whitelist against the request path only, not the full URI with query
parameters (#3737)
- **Governance Blocked Models UI** — Restored the missing Blocked Models
create/edit UI in the VK provider config sheet (#3750)
- **Logging Plugin Cleanup Drain** — Fixed a shutdown race where
`batchWriter` could drop in-flight log entries; `Cleanup` now drains
both the recovered batch and remaining queue within a 30-second budget
(#3717)
- **Model Rankings Empty Entries** — Excluded entries with empty `model`
values from model rankings matview queries so blank rows no longer
surface in the UI (#3758)
- **User Filter Duplicates** — Recreated `mv_filter_users` matview to
require non-empty `user_name`, eliminating duplicate filter dropdown
entries (#3764)
- **User Filter Display Name** — Use `user_name` instead of `user_id` as
the display label for users in logging filters (#3691)
- **Large Numeric ID Precision** — Preserve large numeric IDs in URL
search params by skipping JSON parsing for plain strings (#3692)

## 🔧 Refactors & Chores

- **Error Propagation for GetAvailable\* APIs** — `GetAvailable*`
methods on `LoggerPlugin`/`LogManager` now return wrapped errors instead
of silently logging and returning empty slices (#3759)
- **Governance Blocklist Matching** — Use `slices.Contains` for VK
blocked-model matching for clearer code with identical semantics (#3727)
- **Exported `ResolvePeriod`** — Renamed `resolvePeriod` to
`ResolvePeriod` so external packages can reuse the period parsing
(#3763)

## 📚 Docs

- **OTEL Env Var Documentation** — Documented `env.VAR_NAME` support for
`collector_url`, `metrics_endpoint`, and headers in OTEL/Prometheus
plugin docs
- **OTEL OSS Features & Examples** — Added OTEL documentation to the OSS
features list with usage examples (#3731)
- **Anthropic Auth Recommendation** — Recommend `ANTHROPIC_AUTH_TOKEN`
over `ANTHROPIC_CUSTOM_HEADERS` for Claude Code authentication (#3686)
@akshaydeo akshaydeo mentioned this pull request May 27, 2026
18 tasks
akshaydeo added a commit that referenced this pull request May 27, 2026
## Summary

This PR releases Bifrost OSS `v1.5.5` and Enterprise `v1.4.4`, bumping all module pins from `v1.5.12`/`v1.3.12` to `v1.5.13`/`v1.3.13` across core, framework, and all plugins. It also hardens the Docker manifest shell scripts, expands CI egress allowlists, and updates documentation to reflect the new SCIM-based user provisioning feature.

## Changes

- **Module version bumps**: All `go.mod`/`go.sum` files updated from `core v1.5.12` → `v1.5.13`, `framework v1.3.12` → `v1.3.13`, and all plugin versions incremented accordingly (`compat`, `governance`, `jsonparser`, `logging`, `maxim`, `mocker`, `otel`, `prompts`, `semanticcache`, `telemetry`).
- **Docker manifest scripts**: Added `#!/usr/bin/env bash` shebang and `set -euo pipefail` to `create-docker-manifest.sh` and `create-docker-manifest-ubi9.sh`; quoted all variable expansions and switched `jq -r` to `jq -er` to fail on null digests.
- **CI egress allowlist**: Added `production.cloudfront.docker.com:443` to Docker-related job allowlists, and added `_https._tcp.dl.google.com:443` and `motd.ubuntu.com:443` to the Ubuntu package job allowlist.
- **Changelog files**: Cleared per-module `changelog.md` files (content moved into the new versioned docs). Added `docs/changelogs/v1.5.5.mdx` and `docs/changelogs/ent-v1.4.4.mdx` with full release notes, and registered both in `docs/docs.json`.
- **Documentation**: Replaced the SSO Integration link with a User Provisioning (SCIM) link in both `README.md` and `transports/README.md`.
- **Enterprise v1.4.4 highlights** (documented): Kafka and Google Cloud Pub/Sub observability sinks, chunked streaming with a 100 MB inter-node message ceiling, BigQuery custom labels via env vars using the new `ConfigMarshallerPlugin` interface, temporary access token expiry extensions, and a multi-node cluster integration harness.
- **OSS v1.5.5 highlights** (documented): Azure v1 API migration, env-var support for OTel/Prometheus configs, OTel extra-header forwarding and semantic-convention alignment, virtual key quota including provider configs, Responses API streaming in `jsonparser`, and a batch of Bedrock, Gemini, Anthropic, Azure, and logging plugin fixes.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [x] Documentation
- [x] Chore/CI

## Affected areas

- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [x] Docs

## How to test

```sh
# Core/Transports
go version
go test ./...

# Verify Docker manifest scripts exit on error
bash -n .github/workflows/scripts/create-docker-manifest.sh
bash -n .github/workflows/scripts/create-docker-manifest-ubi9.sh
```

Validate that the new changelog pages (`changelogs/v1.5.5` and `changelogs/ent-v1.4.4`) render correctly in the docs site.

## Screenshots/Recordings

N/A

## Breaking changes

- [x] Yes
- [ ] No

The Azure provider no longer accepts `api_version` in `AzureKeyConfig` and has migrated to the `/openai/v1/{operation}` URL pattern. See the [v1.4.0 Migration Guide](https://docs.getbifrost.ai/enterprise/migration-guides/v1.4.0) for full details.

## Related issues

#3661, #3756, #3651, #3730, #3732, #3754, #3747, #3690, #3729, #3685, #3733, #3761, #3735, #3721, #3720, #3749, #3698, #3762, #3750, #3727, #3717, #3759, #3758, #3764, #3691, #3692, #3737, #3763

## Security considerations

- The `ConfigMarshallerPlugin` interface redacts secrets (OTel collector URLs, Prometheus push gateway credentials, BigQuery labels) at config storage time and rehydrates them at load time, preventing plaintext secret persistence.
- Docker manifest scripts now use `set -euo pipefail`, preventing silent failures that could result in malformed or missing image manifests being pushed.

## 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants