fix(logstore): repair racy has_object waits and stale payload count in tests

[kohlivrinda]

Summary

Test helpers that waited for object uploads were using objStore.Len() as a proxy for upload completion, but processUpload commits the has_object flag to the database only after the object Put succeeds. This meant tests could proceed before the row's has_object flag was set, causing flaky assertions that depend on hydration state.

Changes

• Replaced waitForUploads(t, func() bool { return objStore.Len() == N }) calls with waitForOffload(t, inner, id) (for regular logs) and the new waitForMCPOffload(t, inner, id) (for MCP tool logs), both of which poll the database row's has_object flag directly rather than counting objects in the store.
• Added waitForMCPOffload helper that uses FindMCPToolLog to check HasObject, mirroring the existing waitForOffload helper for standard log entries.
• Fixed the TestHybrid_ContentHiddenBatchMixed test to wait for the visible row's has_object flag before asserting hydration behavior.
• Updated the TestExtractPayload_RoundTrip assertion to account for six identity snapshot fields (provider, model, status, timestamp, selected_key_id, selected_key_name) that ExtractPayload always includes in the payload map, changing the expected count from len(payloadFields)+1 to len(payloadFields)+7.

Type of change

• Bug fix

Affected areas

• Core (Go)

How to test

go test ./framework/logstore/...

All previously flaky tests should now pass consistently without race conditions between the upload goroutine committing has_object and test assertions reading hydration state.

Breaking changes

• No

Security considerations

None.

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]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4951c163-5390-4376-9051-eb3e04375e00

📥 Commits

Reviewing files that changed from the base of the PR and between c40319c and 9e59e07.

📒 Files selected for processing (3)

• framework/logstore/contenthidden_test.go
• framework/logstore/hybrid_test.go
• framework/logstore/payload_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• framework/logstore/payload_test.go
• framework/logstore/contenthidden_test.go
• framework/logstore/hybrid_test.go

---

📝 Walkthrough

Summary by CodeRabbit

• Tests
  • Improved synchronization for content hiding and log hydration scenarios.
  • Updated hybrid log tests to verify completed database offloading across logs, responses, attachments, and speech input.
  • Expanded payload round-trip coverage to include identity snapshot details such as provider, model, status, timestamp, and selected key information.

Walkthrough

The changes update logstore tests to wait for committed database offload flags before asserting hydration-related behavior. The payload round-trip test now accounts for six identity snapshot fields.

Changes

Logstore test synchronization

Layer / File(s)	Summary
Offload state polling framework/logstore/hybrid_test.go	Adds MCP offload polling and retains database-backed log-store handles where synchronization requires them.
Hybrid scenario synchronization framework/logstore/hybrid_test.go	Updates standard logs, MCP logs, responses, attachments, speech input, and failure scenarios to wait for persisted offload state.
Content and payload assertions framework/logstore/contenthidden_test.go, framework/logstore/payload_test.go	Waits for the visible row’s offload flag and expects six identity snapshot fields in the payload size assertion.

Estimated code review effort: 2 (Simple) | ~15 minutes

Merge Risk: ⚪ Minimal · up to 9e59e

This PR updates logstore test waits to observe database hydration state directly and corrects a stale payload-count expectation; no actionable merge-blocking risk remains after normal checks.

Possibly related PRs

• maximhq/bifrost#6070: Adds identity metadata fields that this PR validates in payload tests.
• maximhq/bifrost#6134: Updates the same payload expectation for six identity fields.
• maximhq/bifrost#6142: Validates the same six always-present dimension keys.

Suggested reviewers: madhuvod, akshaydeo

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly identifies the main fixes: racy has_object waits and the stale payload count in logstore tests.
Description check	✅ Passed	The description covers the purpose, changes, testing command, affected area, breaking changes, security, and checklist status.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 08-14-fix_logstore_repair_racy_has_object_waits_and_stale_payload_count_in_tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kohlivrinda]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• feat(routing): LLM classifier fallback for complexity routing #6177
• fix(logstore): repair racy has_object waits and stale payload count in tests #6176 👈 (View in Graphite)
• feat(session aware routing): session observability — log columns, filters, and the sessions configuration UI #6175
• feat(session aware routing): session-sticky targets and cache-aware session tier policy #6174
• feat(session aware routing): session contracts, identity resolution, and the KV session state store #6173
• docs(routing): semantic complexity router helm values, OpenAPI, and documentation #6168
• feat(routing): semantic router API surface and configuration UI #6167
• feat(routing): semantic classification on the request path, with logs, traces, and telemetry #6166
• feat(routing): semantic classifier runtime and harness-aware input extraction #6165
• feat(routing): vector stores, chromem backend, and the semantic embedding executor #6164
• feat(routing): three-tier complexity contracts, semantic config wire, and migrations #6163
• docs: migrate routing rules and complexity analyzer endpoints from /api/governance/* to /api/routing/* with deprecated legacy aliases #6147
• refactor: extract routing rules and complexity analyzer endpoints into dedicated RoutingHandler under /api/routing/* with backwards-compatible /api/governance/* aliases #6146
• refactor: move PublishRoutingAllowlist and LoadBalanceProvider from governance's PreRequestHook into the routing plugin so both run after rule evaluation on the post-rule model #6145
• refactor: extract routing rules and complexity router into a dedicated routing plugin #6144
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.