Skip to content

test(amber): cover AI assistant initialization - #7156

Merged
aglinxinyuan merged 2 commits into
apache:mainfrom
aglinxinyuan:test/ai-assistant-manager
Jul 31, 2026
Merged

test(amber): cover AI assistant initialization#7156
aglinxinyuan merged 2 commits into
apache:mainfrom
aglinxinyuan:test/ai-assistant-manager

Conversation

@aglinxinyuan

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Adds focused unit coverage for the OpenAI assistant handshake: a successful local /models response verifies the GET request and normalized bearer key, while non-success and malformed-URL paths verify the fallback result.

Any related issues, documentation, discussions?

Closes #7152

How was this PR tested?

  • WorkflowExecutionService/testOnly org.apache.texera.web.resource.aiassistant.AiAssistantManagerSpec — 3 succeeded.
  • WorkflowExecutionService/Test/scalafmtCheck and WorkflowExecutionService/Test/scalafix --check.

Mutation proof:

Production mutation Result
Changed the HTTP method from GET to POST 1 test failed
Reversed the responseCode == 200 condition 2 tests failed
Returned OpenAI from the exception fallback 1 test failed

Each mutation was reverted before the final green test run.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Codex (GPT-5)

@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • No candidates found from git blame history.

@aglinxinyuan
aglinxinyuan marked this pull request as ready for review July 31, 2026 04:16
@aglinxinyuan
aglinxinyuan requested a review from mengw15 July 31, 2026 04:16
@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 2 better · 🔴 1 worse · ⚪ 12 noise (<±5%) · 0 without baseline

Compared against main 45b7b20 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🔴 bs=10 sw=10 sl=64 406 0.248 24,375/32,821/32,821 us 🔴 +11.6% / 🔴 +107.9%
🟢 bs=100 sw=10 sl=64 923 0.563 106,345/128,213/128,213 us 🟢 -13.8% / 🔴 +16.2%
bs=1000 sw=10 sl=64 1,098 0.67 905,544/958,101/958,101 us ⚪ within ±5% / 🟢 -10.8%
Baseline details

Latest main 45b7b20 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 406 tuples/sec 422 tuples/sec 771.62 tuples/sec -3.8% -47.4%
bs=10 sw=10 sl=64 MB/s 0.248 MB/s 0.258 MB/s 0.471 MB/s -3.9% -47.3%
bs=10 sw=10 sl=64 p50 24,375 us 21,834 us 12,626 us +11.6% +93.1%
bs=10 sw=10 sl=64 p95 32,821 us 32,772 us 15,786 us +0.1% +107.9%
bs=10 sw=10 sl=64 p99 32,821 us 32,772 us 19,344 us +0.1% +69.7%
bs=100 sw=10 sl=64 throughput 923 tuples/sec 934 tuples/sec 971.56 tuples/sec -1.2% -5.0%
bs=100 sw=10 sl=64 MB/s 0.563 MB/s 0.57 MB/s 0.593 MB/s -1.2% -5.1%
bs=100 sw=10 sl=64 p50 106,345 us 104,269 us 103,463 us +2.0% +2.8%
bs=100 sw=10 sl=64 p95 128,213 us 148,767 us 110,296 us -13.8% +16.2%
bs=100 sw=10 sl=64 p99 128,213 us 148,767 us 118,690 us -13.8% +8.0%
bs=1000 sw=10 sl=64 throughput 1,098 tuples/sec 1,093 tuples/sec 1,001 tuples/sec +0.5% +9.7%
bs=1000 sw=10 sl=64 MB/s 0.67 MB/s 0.667 MB/s 0.611 MB/s +0.4% +9.6%
bs=1000 sw=10 sl=64 p50 905,544 us 918,821 us 1,008,988 us -1.4% -10.3%
bs=1000 sw=10 sl=64 p95 958,101 us 945,299 us 1,055,260 us +1.4% -9.2%
bs=1000 sw=10 sl=64 p99 958,101 us 945,299 us 1,074,689 us +1.4% -10.8%
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,492.87,200,128000,406,0.248,24375.38,32820.58,32820.58
1,100,10,64,20,2167.87,2000,1280000,923,0.563,106345.02,128213.10,128213.10
2,1000,10,64,20,18220.23,20000,12800000,1098,0.670,905544.36,958101.43,958101.43

@codecov-commenter

codecov-commenter commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.62%. Comparing base (45b7b20) to head (7c4c25d).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #7156   +/-   ##
=========================================
  Coverage     79.62%   79.62%           
+ Complexity     3836     3834    -2     
=========================================
  Files          1160     1160           
  Lines         46188    46188           
  Branches       5145     5145           
=========================================
+ Hits          36777    36779    +2     
+ Misses         7778     7771    -7     
- Partials       1633     1638    +5     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø) Carriedforward from 91ed98d
agent-service 77.42% <ø> (ø) Carriedforward from 91ed98d
amber 73.20% <ø> (+0.11%) ⬆️
computing-unit-managing-service 20.49% <ø> (ø) Carriedforward from 91ed98d
config-service 65.97% <ø> (ø) Carriedforward from 91ed98d
file-service 66.80% <ø> (ø) Carriedforward from 91ed98d
frontend 83.08% <ø> (-0.08%) ⬇️ Carriedforward from 91ed98d
notebook-migration-service 78.89% <ø> (ø) Carriedforward from 91ed98d
pyamber 97.31% <ø> (-0.05%) ⬇️ Carriedforward from 91ed98d
workflow-compiling-service 26.31% <ø> (ø) Carriedforward from 91ed98d

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mengw15 mengw15 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.

LGTM

Copilot AI 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.

Pull request overview

Adds a new ScalaTest spec in the Amber backend to unit-test the OpenAI “handshake” logic in AiAssistantManager by exercising the private initOpenAI method against a local in-process HTTP server.

Changes:

  • Introduces AiAssistantManagerSpec to validate the /models probe uses GET and emits a normalized Authorization: Bearer … header.
  • Adds negative-path coverage for non-200 responses and malformed configured service URLs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Jul 31, 2026
Merged via the queue into apache:main with commit e6b000e Jul 31, 2026
21 checks passed
@aglinxinyuan
aglinxinyuan deleted the test/ai-assistant-manager branch July 31, 2026 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add tests for AI assistant initialization

4 participants