Skip to content

chore: final portfolio polish — changelog, ADRs, test hygiene, CI coverage#55

Merged
eulicesl merged 7 commits into
mainfrom
chore/portfolio-final-polish
Apr 18, 2026
Merged

chore: final portfolio polish — changelog, ADRs, test hygiene, CI coverage#55
eulicesl merged 7 commits into
mainfrom
chore/portfolio-final-polish

Conversation

@eulicesl

@eulicesl eulicesl commented Apr 18, 2026

Copy link
Copy Markdown
Owner

Summary

Six portfolio hygiene improvements across docs, tests, and CI. None affect runtime behavior.

# Change Rationale
1 Soften README Conventional Commits claim PRs #18–39 used plain imperative messages; the old wording overstated history. Now reads "Conventional Commits (adopted from v1.0.0 onward)."
2 Add CHANGELOG.md (Keep a Changelog format) No release changelog existed. Covers v1.0.0 Added/Changed/Fixed sections drawn from the release tag and git log. Linked from README.
3 Replace portfolio GitHub topic Swapped for testflight, swiftdata, local-llm — keywords that match how developers search for this stack. (Applied via gh repo edit; no source file.)
4 Fix 6 force unwraps in MemoryStoreTests.swift UserDefaults(suiteName:)! crashes the test runner on failure instead of surfacing a readable assertion. Replaced with a makeTestDefaults(suiteName:) throws helper using try #require (Swift Testing idiom).
5 Add ADRs 0002–0005 Backfills architectural decisions that were implied by the code but never written down: on-device AI default, SwiftData choice, no-backend posture, Keychain for credentials.
6 Enable CI code coverage Adds -enableCodeCoverage YES and -resultBundlePath to the xcodebuild test step, then parses the .xcresult to print Code Coverage: X.X% in the job log. Hard threshold deferred to a follow-on PR.

Test plan

  • CI passes on this PR (build + unit tests)
  • CHANGELOG.md renders correctly on GitHub
  • ADR links in docs/adr/README.md resolve
  • MemoryStoreTests — no force-unwrap crash paths remain (grep -c '!' LumenTests/MemoryStoreTests.swift should show only logical-not uses)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Comprehensive CHANGELOG added documenting v1.0.0 release with features, improvements, and bug fixes
    • Architecture decision records added documenting system design choices for AI runtime, data persistence, credential storage, and architecture
    • Updated repository documentation with engineering practices clarification and changelog references

…tory

PRs #18–39 used plain imperative messages; conventional commits were
adopted consistently from v1.0.0 onward.
Covers v1.0.0 with Added/Changed/Fixed sections pulled from the
release tag message and git log.
Add makeTestDefaults(suiteName:) helper using try #require so a missing
suite surfaces a clear test failure instead of a crash. All six
AppStoreSecurityTests functions are now throws.
- 0002: on-device-first AI with Apple FoundationModels (privacy rationale, Ollama fallback)
- 0003: SwiftData over Core Data (modern persistence, migration story)
- 0004: no backend / serverless architecture (privacy-by-design, no data liability)
- 0005: Keychain for credential storage (over UserDefaults, includes migration backstory)
Add -enableCodeCoverage YES and -resultBundlePath to the xcodebuild
test step, then parse the .xcresult with xccov to print coverage% in
the job log. Hard threshold is deferred to a follow-on PR.
@coderabbitai

coderabbitai Bot commented Apr 18, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

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

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 8 minutes and 45 seconds.

⌛ 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: ASSERTIVE

Plan: Pro

Run ID: 72aac5e0-893f-4a0a-a9b3-5f2ae5860506

📥 Commits

Reviewing files that changed from the base of the PR and between ab81fff and be4d0d2.

📒 Files selected for processing (5)
  • docs/adr/0002-on-device-first-ai-with-apple-foundation-models.md
  • docs/adr/0003-swiftdata-over-core-data.md
  • docs/adr/0004-no-backend-serverless-architecture.md
  • docs/adr/0005-keychain-for-credential-storage.md
  • docs/adr/README.md
📝 Walkthrough

Walkthrough

Very good, sir. This pull request prepares the Lumen application for its 1.0.0 public release. It introduces code coverage instrumentation to the CI pipeline, establishes formal release documentation via a comprehensive changelog, implements four new Architecture Decision Records articulating critical design choices regarding on-device AI inference, persistence mechanisms, serverless deployment, and credential management, and refines test suite error handling to employ proper requirement assertions.

Changes

Cohort / File(s) Summary
CI & Code Coverage
.github/workflows/ci.yml
Adds Xcode code coverage collection (-enableCodeCoverage YES) and implements a new "Report code coverage" step that parses coverage artifacts via xcrun xccov and a Python script, with fallback to unavailable status.
Release Documentation
CHANGELOG.md, README.md
Adds comprehensive v1.0.0 release notes in standardized Keep a Changelog format; updates README to reference CHANGELOG.md for TestFlight notes and clarifies Conventional Commits adoption from v1.0.0 onward.
Architecture Decision Records
docs/adr/0002-on-device-first-ai-with-apple-foundation-models.md, docs/adr/0003-swiftdata-over-core-data.md, docs/adr/0004-no-backend-serverless-architecture.md, docs/adr/0005-keychain-for-credential-storage.md, docs/adr/README.md
Establishes four new ADRs documenting core design decisions: Apple FoundationModels as default AI runtime, SwiftData for on-device persistence, serverless architecture without backend infrastructure, and Keychain-exclusive credential storage. Updates ADR index accordingly.
Test Suite Enhancement
LumenTests/MemoryStoreTests.swift
Introduces private helper makeTestDefaults(suiteName:) and converts six test methods to throwing variants, replacing force-unwrapped UserDefaults initialization with proper requirement assertions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Poem

🏰 One point oh approaches with grace,
Coverage reports take their place,
Four pillars of architecture stand tall—
On-device, serverless, secure through it all. 🔐
Documentation complete, the foundation's now sound.

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)
Check name Status Explanation
Title check ✅ Passed The title concisely summarizes the main changes: portfolio polish across changelog, architectural decisions, test improvements, and CI coverage.
Description check ✅ Passed The description is well-structured with a comprehensive summary table, clear rationale for each change, and a test plan, though some template sections are not filled (Linked Issues, Evidence checkboxes, Human Verification details).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/portfolio-final-polish

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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request adds a CHANGELOG.md for the 1.0.0 release, updates the README.md with project details, and introduces several Architecture Decision Records (ADRs) covering on-device AI, SwiftData, and security. Unit tests were refactored to use Swift Testing's #require for improved safety. Feedback was provided regarding a discrepancy in ADR 0005, where the documented Keychain cleanup logic does not align with the actual implementation.

Comment thread docs/adr/0005-keychain-for-credential-storage.md Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/adr/0002-on-device-first-ai-with-apple-foundation-models.md`:
- Line 36: Update the vague Apple reference in
docs/adr/0002-on-device-first-ai-with-apple-foundation-models.md by replacing
the phrase "Apple `FoundationModels` framework documentation (WWDC 2025)" with a
precise citation: include the WWDC session number and title and add the session
URL (or Apple developer docs URL) so future readers can locate the source
directly; edit the line containing "Apple `FoundationModels` framework
documentation (WWDC 2025)" to append or substitute the specific session
identifier and link.

In `@docs/adr/0005-keychain-for-credential-storage.md`:
- Around line 24-26: The ADR claims Keychain items are purged on first launch
after reinstall but AppStore.swift currently detects first-launch (around the
first-launch detection in AppStore.swift lines ~47–50) without calling the
existing removal routine; add an explicit purge during initialization that calls
the existing removeValue() for ollamaLocalBearerTokenKey and
ollamaCloudAPIKeyKey before any load/migration logic runs, and extend
AppStoreSecurityTests to cover the fresh-install purge scenario (simulate fresh
install state and assert keys are removed/not loaded); alternatively, if you
prefer not to implement purge, update the ADR text to remove the “app deletes
its Keychain items on first launch after re-install” claim and reference
AppStore.swift and AppStoreSecurityTests accordingly.

In `@docs/adr/README.md`:
- Around line 61-64: The ADR index rows for
[0002](0002-on-device-first-ai-with-apple-foundation-models.md),
[0003](0003-swiftdata-over-core-data.md),
[0004](0004-no-backend-serverless-architecture.md) and
[0005](0005-keychain-for-credential-storage.md) show "Accepted 2025-10-01" which
conflicts with ADRs 0000/0001 (Accepted 2026-04-16); update these entries in
docs/adr/README.md to either set their status to "Proposed" if they are not yet
merged, or change the acceptance date to the actual acceptance date (or add a
parenthetical note stating they are retrospective) so the audit trail is
consistent and unambiguous.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: 7e0df181-f2f3-4fa9-a871-2daa1cb1e753

📥 Commits

Reviewing files that changed from the base of the PR and between caa8682 and ab81fff.

📒 Files selected for processing (9)
  • .github/workflows/ci.yml
  • CHANGELOG.md
  • LumenTests/MemoryStoreTests.swift
  • README.md
  • docs/adr/0002-on-device-first-ai-with-apple-foundation-models.md
  • docs/adr/0003-swiftdata-over-core-data.md
  • docs/adr/0004-no-backend-serverless-architecture.md
  • docs/adr/0005-keychain-for-credential-storage.md
  • docs/adr/README.md

Comment thread docs/adr/0002-on-device-first-ai-with-apple-foundation-models.md Outdated
Comment thread docs/adr/0005-keychain-for-credential-storage.md Outdated
Comment thread docs/adr/README.md Outdated
eulicesl and others added 2 commits April 18, 2026 14:44
- ADR 0005: remove unimplemented first-launch Keychain purge claim from
  Neutral consequences; AppStore.swift detects isFirstLaunch but does
  not call removeValue on that path
- ADR 0002: replace vague WWDC 2025 reference with Meet Foundation Models
  session title and documentation URL
- ADR 0002-0005: mark status as retrospective to explain date predating
  the ADR practice established in 0000 (2026-04-16)
@eulicesl
eulicesl merged commit 2b0f27b into main Apr 18, 2026
5 checks passed
@eulicesl
eulicesl deleted the chore/portfolio-final-polish branch April 18, 2026 18:56
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