Skip to content

[FIX] Stub the lookups seam in the profile-resolution tests - #2228

Merged
hari-kuriakose merged 1 commit into
mainfrom
fix/lookups-seam-test
Aug 6, 2026
Merged

[FIX] Stub the lookups seam in the profile-resolution tests#2228
hari-kuriakose merged 1 commit into
mainfrom
fix/lookups-seam-test

Conversation

@chandrasekharan-zipstack

@chandrasekharan-zipstack chandrasekharan-zipstack commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

What

  • Patch get_lookup_config in test_profile_resolution_fallback.py's _call helper, alongside the collaborators it already stubs.
  • Assert the seam was called with the prompt, for the single-prompt builder.

Why

  • _call patches every collaborator build_fetch_response_payload reaches except get_lookup_config (prompt_studio_helper.py:903).
  • prompt_studio/lookup_utils.py no-ops when pluggable_apps.lookups is absent, so the suite is green in OSS. In trees where the lookups plugin is installed, the real build_lookup_config_for_prompt runs PromptLookupAssignment.objects.filter(prompt=prompt) against the MagicMock prompt and raises django.core.exceptions.ValidationError: ['"[]" is not a valid UUID.'].
  • 4 tests fail there today — the three TestProfileResolutionLadder cases plus TestResolvedProfileReachesCallback, all on the [build_fetch_response_payload] param.

How

  • patch.object(psh, "get_lookup_config", autospec=True, return_value=None) added to the existing patches list, so both trees take the identical path.
  • psh.get_lookup_config.assert_called_once_with(prompt) after the builder returns, so stubbing cannot silently mask a removed call site.
  • Scoped to build_fetch_response_payload: build_bulk_fetch_response_payload has no lookup call site, which is why it was unaffected.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

  • No. Test-only change, confined to one file. No production code touched.

Database Migrations

  • None.

Env Config

  • None.

Relevant Docs

  • N/A

Related Issues or PRs

Dependencies Versions

  • Unchanged.

Notes on Testing

Run against a tree with the lookups plugin merged in, so the seam is live:

Run Result
Plugin present, without this change 4 failed, 9 passed — reproduces the failure exactly
Plugin present, with this change 13 passed
With this change + get_lookup_config call site deleted from the helper 4 failed — the new assertion catches it

ruff check clean on the file.

Screenshots

Checklist

I have read and understood the Contribution Guidelines.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NfGKCX7QYPc2P2iaSoskds

@chandrasekharan-zipstack @claude
[FIX] Stub the lookups seam in the profile-resolution tests
9cdad04 
The tests patch every collaborator `build_fetch_response_payload` calls
except `get_lookup_config`. That is a no-op in OSS, so the suite is green
here, but in trees where the lookups plugin is installed it runs a real
ORM query against the mock prompt and raises `ValidationError`.

Stub the seam so both trees take the same path, and assert it was called
with the prompt so the stub cannot hide a deleted call site.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NfGKCX7QYPc2P2iaSoskds
@coderabbitai

coderabbitai Bot commented Aug 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5fdd52d1-6e35-42e5-a8e3-1cd66a7d2691

📥 Commits

Reviewing files that changed from the base of the PR and between 6be8b55 and 9cdad04.

📒 Files selected for processing (1)
  • backend/prompt_studio/prompt_studio_core_v2/tests/test_profile_resolution_fallback.py

Summary by CodeRabbit

  • Tests
    • Improved test coverage and reliability for prompt profile resolution.
    • Added validation that prompt lookup configuration is requested correctly when building a single prompt.
    • No end-user-facing functionality or behavior changed.

Walkthrough

The test helper now stubs get_lookup_config and verifies that build_fetch_response_payload calls it once with the prompt while preserving the generated result.

Changes

Profile resolution fallback tests

Layer / File(s) Summary
Isolate and verify lookup configuration
backend/prompt_studio/prompt_studio_core_v2/tests/test_profile_resolution_fallback.py		 The helper stubs get_lookup_config with None, captures the payload builder result, and asserts the expected single lookup call.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: hari-kuriakose

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main test change: stubbing the lookups seam in profile-resolution tests.
Description check ✅ Passed The description follows the repository template and explains the change, rationale, testing, impact, and configuration status.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ 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 fix/lookups-seam-test

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

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

@sonarqubecloud

sonarqubecloud Bot commented Aug 6, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This test-only change isolates profile-resolution tests from the optional lookups plugin while ensuring the single-prompt builder still invokes the lookup seam.

  • Patches get_lookup_config alongside the helper’s existing collaborators.
  • Asserts that build_fetch_response_payload calls the seam once with the tested prompt.

Confidence Score: 5/5

The PR appears safe to merge because the change is confined to tests and preserves the intended profile-resolution coverage.

The patch targets the correct module binding, remains active through the builder invocation, returns the same value as the OSS no-plugin path, and applies its call assertion only to the intended single-prompt builder.

Important Files Changed
Filename Overview
backend/prompt_studio/prompt_studio_core_v2/tests/test_profile_resolution_fallback.py Correctly stubs the optional lookup integration and verifies the expected single-prompt call without changing production behavior.

Reviews (1): Last reviewed commit: "[FIX] Stub the lookups seam in the profi..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Unstract test results

Per-group results

Status Group Tier Passed Failed Errors Skipped Duration (s)
✅ e2e-api-deployment e2e 3 0 0 0 20.4
✅ e2e-coowners e2e 1 0 0 0 1.2
✅ e2e-etl e2e 1 0 0 0 8.1
✅ e2e-login e2e 2 0 0 0 1.0
✅ e2e-prompt-studio e2e 1 0 0 0 4.4
✅ e2e-smoke e2e 2 0 0 0 0.9
✅ e2e-workflow e2e 1 0 0 0 17.7
✅ integration-backend integration 205 0 0 26 40.5
✅ integration-connectors integration 1 0 0 7 7.7
✅ integration-workers integration 140 0 0 1 48.3
✅ unit-backend unit 460 0 0 1 39.7
✅ unit-connectors unit 63 0 0 0 10.6
✅ unit-core unit 33 0 0 0 1.5
✅ unit-platform-service unit 15 0 0 0 2.8
✅ unit-rig unit 109 0 0 0 5.6
✅ unit-sdk1 unit 480 0 0 0 24.8
✅ unit-workers unit 1335 0 0 1 103.5
TOTAL 2852 0 0 36 338.6

Critical paths

⚠️ Critical paths not yet covered

  • workflow-execution-fan-out — Multi-file workflow execution fans out to file-processing workers and rejoins. (declared coverage: no groups declared)
✅ Covered critical paths
  • auth-login — covered by e2e-login
  • adapter-register-llm — covered by integration-backend
  • workflow-author — covered by integration-backend
  • co-owner-manage — covered by integration-backend, e2e-coowners
  • workflow-create-execute — covered by e2e-workflow
  • api-deployment-provision — covered by integration-backend
  • api-deployment-auth — covered by integration-backend
  • api-deployment-run — covered by e2e-api-deployment
  • prompt-studio-author — covered by integration-backend
  • prompt-studio-fetch-response — covered by e2e-prompt-studio
  • connector-register-test — covered by integration-backend
  • pipeline-etl-execute — covered by e2e-etl
  • usage-aggregate-read — covered by integration-backend
  • usage-token-tracking — covered by e2e-api-deployment
  • callback-result-delivery — covered by e2e-api-deployment

@hari-kuriakose
hari-kuriakose merged commit f271d5c into main Aug 6, 2026
11 checks passed
@hari-kuriakose
hari-kuriakose deleted the fix/lookups-seam-test branch August 6, 2026 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] approved these changes

Assignees

@chandrasekharan-zipstack chandrasekharan-zipstack

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@chandrasekharan-zipstack @hari-kuriakose