Skip to content

try android CI with .when and add some relationship required basics#23

Merged
MiaKoring merged 8 commits into
amethystsoft:mainfrom
MiaKoring:feat/relationship
May 19, 2026
Merged

try android CI with .when and add some relationship required basics#23
MiaKoring merged 8 commits into
amethystsoft:mainfrom
MiaKoring:feat/relationship

Conversation

@MiaKoring
Copy link
Copy Markdown
Member

@MiaKoring MiaKoring commented May 19, 2026

Summary

This PR introduces Android CI support and refactors dependency management with conditional platform targeting, along with new relationship model lookup capabilities.

Changes

GitHub Actions Workflow

  • Added new .github/workflows/swift-test-android.yml to test Swift packages on Android via skiptools/swift-android-action@v2
  • Configured with free-disk-space: true to mitigate emulator installation failures on space-constrained runners

Dependency Management Improvements

  • Updated Package.swift to use .product(...) entries with condition: .when(platforms: ...) for cleaner conditional dependency declaration
  • Replaced imperative conditional compilation blocks with declarative platform conditions
  • Consolidated ULID, KeychainAccess (Apple platforms: iOS, macOS, tvOS, visionOS, watchOS), and KeyringAccess (Linux) dependencies within the main array

Relationship Model Support

  • Added ManagedObjectContext.getModel(id:type:) method to retrieve persisted models by ULID
  • Implements intelligent lookup: first checks the context's identity map for tracked instances, then performs a fetch if needed
  • Method properly throws MOCError for error handling

Platform-Specific Key Retrieval

  • Enhanced ManagedObjectContext.getKeyForDatabase() with explicit Linux platform branch using KeyringAccess
  • Added fallback for unsupported platforms returning nil

Highlights

The adoption of Swift's .when() conditional syntax for dependency declaration is a significant improvement—it moves from imperative conditional logic to declarative platform targeting, making the Package.swift manifest more maintainable and aligned with modern Swift package conventions. This pattern is particularly elegant given the complexity of supporting multiple Apple platforms and Linux.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

Warning

Rate limit exceeded

@MiaKoring has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 21 minutes and 5 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e7758d9d-be0f-4426-9dfd-47f91985f40f

📥 Commits

Reviewing files that changed from the base of the PR and between b51488e and 88b4aa9.

📒 Files selected for processing (3)
  • .github/workflows/swift-test-android.yml
  • .github/workflows/swift-test-linux.yml
  • .github/workflows/swift-test-mac.yml
📝 Walkthrough

Walkthrough

This PR adds ULID-based model retrieval by introducing a public getModel(id:type:) API to ManagedObjectContext, updates Package.swift to declare ULID and platform-conditional key-access dependencies using structured conditional products, clarifies platform-specific key retrieval logic for Linux and unsupported platforms, and establishes an Android CI workflow.

Changes

Core Features and Platform Support

Layer / File(s) Summary
Linux Platform Support: Dependencies and Key Retrieval
Package.swift, Sources/Vein/ModelContext/ManagedObjectContext/ManagedObjectContext.swift
veinDependencies now declares ULID directly and uses .product(condition: .when(platforms: ...)) for conditional KeychainAccess (Apple) and KeyringAccess (Linux) entries. getKeyForDatabase(at:appID:) explicitly routes to KeyringAccess on Linux via #elseif os(Linux) and returns nil for unsupported platforms.
ULID-based Model Retrieval
Sources/Vein/ModelContext/ManagedObjectContext/ManagedObjectContext+Relationship.swift
New public method getModel(id:type:) retrieves PersistentModel instances by ULID: checks identity map first, then fetches using predicate matching on the "id" field, returning the first match or nil while propagating MOCError.

Android CI Infrastructure

Layer / File(s) Summary
Android Test Workflow
.github/workflows/swift-test-android.yml
New GitHub Actions workflow "Swift Test (Android)" runs on main branch pushes, PRs targeting main, and manual dispatch; executes skiptools/swift-android-action@v2 on ubuntu-latest with free-disk-space: true to mitigate low-space runner failures.

Possibly related PRs

  • amethystsoft/vein#21: Updates to ManagedObjectContext.getKeyForDatabase to use Linux KeyringAccess key path align with broader Linux DB encryption work.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the two main changes: adding Android CI testing with .when() conditionals and implementing relationship retrieval basics.
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 unit tests (beta)
  • Create PR with unit 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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
Sources/Vein/ModelContext/ManagedObjectContext/ManagedObjectContext+Relationship.swift (1)

4-10: ⚡ Quick win

Database fetch pattern does not optimize for single-record lookups.

The current implementation uses fetchAll().first which executes the full query and returns an array before selecting the first element. While the id predicate ensures only one match, the underlying _fetchAll doesn't apply a LIMIT 1 optimization at the SQL level. No specialized single-record fetch method currently exists in the codebase (no fetchOne, fetchFirst, or similar). If this becomes a performance concern, consider implementing a dedicated single-record fetch method.

🤖 Prompt for 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.

In
`@Sources/Vein/ModelContext/ManagedObjectContext/ManagedObjectContext`+Relationship.swift
around lines 4 - 10, The getModel method currently calls fetchAll(...).first
which materializes a full result set; add a dedicated single-record fetch to
avoid that: implement a new fetchOne/fetchFirst function (e.g., fetchOne<T:
PersistentModel>(_: PredicateBuilder<T>)) that reuses the underlying query
execution path but sets a LIMIT 1 (or uses the lower-level
_fetchAll/_executeQuery with a limit flag), and change
ManagedObjectContext.getModel(id:type:) to call
fetchOne(PredicateBuilder<T>().addCheck(.isEqualTo, "id", id)) instead of
fetchAll(...).first; ensure the new fetchOne returns an optional T? and bubbles
MOCError the same way as fetchAll so callers and identityMap logic remain
unchanged.
🤖 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 @.github/workflows/swift-test-android.yml:
- Around line 12-14: The workflow uses mutable tags for actions and leaves
checkout credentials persisted; replace the tag references actions/checkout@v6
and skiptools/swift-android-action@v2 with their corresponding immutable commit
SHAs (pin to specific commit SHAs) and update the checkout step to disable
credential persistence by adding persist-credentials: false to the
actions/checkout invocation; ensure the pinned SHA strings are used exactly
where actions/checkout and skiptools/swift-android-action are referenced so the
workflow runs a fixed, auditable revision.

---

Nitpick comments:
In
`@Sources/Vein/ModelContext/ManagedObjectContext/ManagedObjectContext`+Relationship.swift:
- Around line 4-10: The getModel method currently calls fetchAll(...).first
which materializes a full result set; add a dedicated single-record fetch to
avoid that: implement a new fetchOne/fetchFirst function (e.g., fetchOne<T:
PersistentModel>(_: PredicateBuilder<T>)) that reuses the underlying query
execution path but sets a LIMIT 1 (or uses the lower-level
_fetchAll/_executeQuery with a limit flag), and change
ManagedObjectContext.getModel(id:type:) to call
fetchOne(PredicateBuilder<T>().addCheck(.isEqualTo, "id", id)) instead of
fetchAll(...).first; ensure the new fetchOne returns an optional T? and bubbles
MOCError the same way as fetchAll so callers and identityMap logic remain
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: e2180f67-035b-46e2-80cb-30b914d0ce1f

📥 Commits

Reviewing files that changed from the base of the PR and between ab2e213 and b51488e.

📒 Files selected for processing (4)
  • .github/workflows/swift-test-android.yml
  • Package.swift
  • Sources/Vein/ModelContext/ManagedObjectContext/ManagedObjectContext+Relationship.swift
  • Sources/Vein/ModelContext/ManagedObjectContext/ManagedObjectContext.swift

Comment thread .github/workflows/swift-test-android.yml Outdated
@MiaKoring MiaKoring merged commit 43531cb into amethystsoft:main May 19, 2026
4 checks passed
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.

1 participant