Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Structural improvements to SpeziLLM #45

Merged
merged 26 commits into from
Feb 23, 2024
Merged

Conversation

philippzagar
Copy link
Member

@philippzagar philippzagar commented Feb 6, 2024

Structural improvements to SpeziLLM

♻️ Current situation & Problem

As of now, when adjusting LLMonFHIR and SpeziFHIR, we noticed some structural limitations of the SpeziLLM library.

⚙️ Release Notes

  • Structural improvements to SpeziLLM, providing a different abstraction methodology for working with LLMs.

📚 Documentation

Proper documentation is written for all components.

✅ Testing

Proper manual testing as well as UI tests

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@philippzagar philippzagar added the enhancement New feature or request label Feb 6, 2024
@philippzagar philippzagar self-assigned this Feb 6, 2024
@PSchmiedmayer PSchmiedmayer self-requested a review February 6, 2024 18:56
Copy link

codecov bot commented Feb 6, 2024

Codecov Report

Attention: Patch coverage is 23.66803% with 745 lines in your changes are missing coverage. Please review.

Project coverage is 31.50%. Comparing base (94c1f3b) to head (65b9bb5).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #45      +/-   ##
==========================================
- Coverage   32.82%   31.50%   -1.32%     
==========================================
  Files          56       66      +10     
  Lines        2322     2750     +428     
==========================================
+ Hits          762      866     +104     
- Misses       1560     1884     +324     
Files Coverage Δ
Sources/SpeziLLM/LLMPlatform.swift 100.00% <100.00%> (ø)
Sources/SpeziLLM/LLMPlatformBuilder.swift 100.00% <100.00%> (ø)
Sources/SpeziLLM/LLMState.swift 52.64% <ø> (ø)
Sources/SpeziLLM/Mock/LLMMockPlatform.swift 100.00% <100.00%> (ø)
Sources/SpeziLLM/Mock/LLMMockSchema.swift 100.00% <100.00%> (ø)
Sources/SpeziLLM/Views/LLMChatViewSchema.swift 100.00% <100.00%> (ø)
...eziLLMLocal/Configuration/LLMLocalParameters.swift 0.00% <ø> (ø)
.../Configuration/LLMLocalPlatformConfiguration.swift 100.00% <ø> (ø)
Sources/SpeziLLMLocal/LLMLocalError.swift 0.00% <ø> (ø)
...Configuration/LLMOpenAIPlatformConfiguration.swift 100.00% <100.00%> (ø)
... and 39 more

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94c1f3b...65b9bb5. Read the comment docs.

@philippzagar philippzagar marked this pull request as ready for review February 8, 2024 06:12
Package.swift Outdated Show resolved Hide resolved
Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

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

Great job with the PR and the improvements @philippzagar! I only had a few comments here and there that would be great if they can be addressed. The semaphore could be addressed in a separate PR as a follow up once the Bluetooth code is merged and elements might have moved over to Spezi Foundation.

Package.swift Outdated Show resolved Hide resolved
Package.swift Show resolved Hide resolved
README.md Show resolved Hide resolved
Sources/SpeziLLM/LLMPlatform.swift Outdated Show resolved Hide resolved
Sources/SpeziLLM/LLMRunner.swift Show resolved Hide resolved
Sources/SpeziLLM/SpeziLLM.docc/SpeziLLM.md Outdated Show resolved Hide resolved
Sources/SpeziLLM/Views/LLMChatView.swift Outdated Show resolved Hide resolved
Sources/SpeziLLMLocal/SpeziLLMLocal.docc/SpeziLLMLocal.md Outdated Show resolved Hide resolved
@philippzagar philippzagar enabled auto-merge (squash) February 23, 2024 01:23
@philippzagar philippzagar merged commit 6892c5d into main Feb 23, 2024
10 checks passed
@philippzagar philippzagar deleted the feat/structural-improvments branch February 23, 2024 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants