Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154) - #414

Merged
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding
Jul 1, 2026
Merged

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154)#414
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding

Conversation

@leogdion

Copy link
Copy Markdown
Member

Re-scope note

Issue #154 cited CustomFieldValue.swift / CustomFieldValuePayload.swiftthose files no longer exist on v1.0.0-beta.3. The remaining actionable hand-written // swiftlint:disable:next cyclomatic_complexity was in FieldValue+Codable.swift (encodeValue(to:)). The other disables live in generatedOperations.*.Output.swift and are out of scope.

What

Split encodeValue(to:) into a thin dispatcher + encodeScalar/encodeComplex helpers so each stays under the complexity threshold and the disable is removed. Behavior, public API, and serialization output unchanged (date→ms math preserved).

Closes#154.

Verification

swift build, full swift test (538 pass), swiftlint --strict clean with the disable removed.

@coderabbitai

coderabbitaiBot commented Jun 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3bc550da-9ce0-438b-b9a8-a07362de7942

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/cyclomatic-complexity-encoding

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.

@codecov

codecovBot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.26%. Comparing base (7791b09) to head (d9e65bb).

Files with missing linesPatch %Lines
...istKit/Models/FieldValues/FieldValue+Codable.swift50.00%10 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## v1.0.0-beta.3 #414 +/- ##
=================================================
+ Coverage 73.85% 74.26% +0.40% 
=================================================
Files 156 156 Lines 3698 3718 +20 =================================================
+ Hits 2731 2761 +30 + Misses 967 957 -10 
FlagCoverage Δ
mistdemo-spm-macos11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-jammy11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-noble11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.3-jammy11.51% <50.00%> (+0.31%)⬆️
mistdemo-swift-6.3-noble11.40% <50.00%> (+0.20%)⬆️
spm72.24% <50.00%> (+0.66%)⬆️
swift-6.1-jammy72.10% <50.00%> (+0.50%)⬆️
swift-6.1-noble72.42% <50.00%> (+0.77%)⬆️
swift-6.2-jammy72.12% <50.00%> (+0.52%)⬆️
swift-6.2-noble72.12% <50.00%> (+0.28%)⬆️
swift-6.3-jammy72.07% <50.00%> (+0.50%)⬆️
swift-6.3-noble72.04% <50.00%> (+0.50%)⬆️

Flags with carried forward coverage won't be shown. 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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@claude

claudeBot commented Jun 30, 2026

Copy link
Copy Markdown

Code Review

Summary: Splits encodeValue(to:) into encodeScalar + encodeComplex to bring cyclomatic complexity under the SwiftLint threshold, removing the // swiftlint:disable:next comment.


Strengths

  • Behaviour is identical — the date→ms math, byte encoding, and all other cases are preserved exactly.
  • The defensive default: throw EncodingError.invalidValue(...) in encodeComplex is exactly the right safety net: if a new FieldValue case is added in the future and the author forgets to route it, the failure is loud and immediate rather than a silent no-op.
  • encodeValue now reads as a clean two-step dispatcher, which is easier to scan at a glance.

Minor observations

Bool return from encodeScalar

The Bool signal ("did I handle it?") works, but the pattern is slightly implicit for a new reader. A name like encodeIfScalar (returning Bool) or restructuring so encodeScalar handles only its own cases and encodeComplex is always called in the else-branch would be equally clear alternatives. Not a change request — just noting the tradeoff.

Comment on encodeScalar explains WHAT, not WHY

/// Encode the scalar cases (string, bytes, int64, double, date).
///
/// - Returns: `true` when `self` was a scalar case…

Per the project's CLAUDE.md, comments should explain the non-obvious WHY. The doc comment's Returns: note is useful because the Bool return is surprising at the call site, so this is borderline acceptable. But the first line is redundant with the function name and the switch body.

default: return false in encodeScalar

This is correct, but since FieldValue is likely not @frozen, the compiler won't warn if a new case is added without handling it here. The fail-loud throw in encodeComplex catches this at runtime; adding an #if DEBUG assertion or a comment explaining why encodeComplex is the backstop would make the intent explicit.


Overall: Minimal, correct, and closes the lint issue without any behavior change. The defensive default path in encodeComplex is a nice touch. LGTM.

…disable (#154)
Re-scoped from the now-removed CustomFieldValue(Payload).swift to the
remaining hand-written disable in FieldValue+Codable.swift. Extracted
helpers so the cyclomatic_complexity swiftlint disable can be removed;
behavior and serialization output unchanged. Generated Operations.*.Output
disables are out of scope.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdionforce-pushed the refactor/cyclomatic-complexity-encoding branch from da921b1 to f28213bCompareJuly 1, 2026 00:48
The #154 refactor split encodeValue into encodeScalar/encodeComplex but
added no tests; the only encode-path test covered .string. Add a
parameterized encode→decode round-trip over the scalar and complex cases
(driving the encodeScalar→encodeComplex delegation), plus explicit
encode-shape assertions for .date (milliseconds) and .bytes (string
payload), which do not round-trip by design.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdion marked this pull request as ready for review July 1, 2026 15:03
@leogdion
leogdion merged commit e2619d0 into v1.0.0-beta.3Jul 1, 2026
70 of 71 checks passed
@leogdion
leogdion deleted the refactor/cyclomatic-complexity-encoding branch July 1, 2026 15:16
@claudeclaudeBot mentioned this pull request Jul 1, 2026
@claudeclaudeBot mentioned this pull request Aug 20, 2026
Sign up for freeto 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

@leogdion
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154) - #414

Merged
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding
Jul 1, 2026
Merged

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154)#414
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding

Conversation

@leogdion

Copy link
Copy Markdown
Member

Re-scope note

Issue #154 cited CustomFieldValue.swift / CustomFieldValuePayload.swiftthose files no longer exist on v1.0.0-beta.3. The remaining actionable hand-written // swiftlint:disable:next cyclomatic_complexity was in FieldValue+Codable.swift (encodeValue(to:)). The other disables live in generatedOperations.*.Output.swift and are out of scope.

What

Split encodeValue(to:) into a thin dispatcher + encodeScalar/encodeComplex helpers so each stays under the complexity threshold and the disable is removed. Behavior, public API, and serialization output unchanged (date→ms math preserved).

Closes#154.

Verification

swift build, full swift test (538 pass), swiftlint --strict clean with the disable removed.

@coderabbitai

coderabbitaiBot commented Jun 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3bc550da-9ce0-438b-b9a8-a07362de7942

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/cyclomatic-complexity-encoding

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.

@codecov

codecovBot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.26%. Comparing base (7791b09) to head (d9e65bb).

Files with missing linesPatch %Lines
...istKit/Models/FieldValues/FieldValue+Codable.swift50.00%10 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## v1.0.0-beta.3 #414 +/- ##
=================================================
+ Coverage 73.85% 74.26% +0.40% 
=================================================
Files 156 156 Lines 3698 3718 +20 =================================================
+ Hits 2731 2761 +30 + Misses 967 957 -10 
FlagCoverage Δ
mistdemo-spm-macos11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-jammy11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-noble11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.3-jammy11.51% <50.00%> (+0.31%)⬆️
mistdemo-swift-6.3-noble11.40% <50.00%> (+0.20%)⬆️
spm72.24% <50.00%> (+0.66%)⬆️
swift-6.1-jammy72.10% <50.00%> (+0.50%)⬆️
swift-6.1-noble72.42% <50.00%> (+0.77%)⬆️
swift-6.2-jammy72.12% <50.00%> (+0.52%)⬆️
swift-6.2-noble72.12% <50.00%> (+0.28%)⬆️
swift-6.3-jammy72.07% <50.00%> (+0.50%)⬆️
swift-6.3-noble72.04% <50.00%> (+0.50%)⬆️

Flags with carried forward coverage won't be shown. 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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@claude

claudeBot commented Jun 30, 2026

Copy link
Copy Markdown

Code Review

Summary: Splits encodeValue(to:) into encodeScalar + encodeComplex to bring cyclomatic complexity under the SwiftLint threshold, removing the // swiftlint:disable:next comment.


Strengths

  • Behaviour is identical — the date→ms math, byte encoding, and all other cases are preserved exactly.
  • The defensive default: throw EncodingError.invalidValue(...) in encodeComplex is exactly the right safety net: if a new FieldValue case is added in the future and the author forgets to route it, the failure is loud and immediate rather than a silent no-op.
  • encodeValue now reads as a clean two-step dispatcher, which is easier to scan at a glance.

Minor observations

Bool return from encodeScalar

The Bool signal ("did I handle it?") works, but the pattern is slightly implicit for a new reader. A name like encodeIfScalar (returning Bool) or restructuring so encodeScalar handles only its own cases and encodeComplex is always called in the else-branch would be equally clear alternatives. Not a change request — just noting the tradeoff.

Comment on encodeScalar explains WHAT, not WHY

/// Encode the scalar cases (string, bytes, int64, double, date).
///
/// - Returns: `true` when `self` was a scalar case…

Per the project's CLAUDE.md, comments should explain the non-obvious WHY. The doc comment's Returns: note is useful because the Bool return is surprising at the call site, so this is borderline acceptable. But the first line is redundant with the function name and the switch body.

default: return false in encodeScalar

This is correct, but since FieldValue is likely not @frozen, the compiler won't warn if a new case is added without handling it here. The fail-loud throw in encodeComplex catches this at runtime; adding an #if DEBUG assertion or a comment explaining why encodeComplex is the backstop would make the intent explicit.


Overall: Minimal, correct, and closes the lint issue without any behavior change. The defensive default path in encodeComplex is a nice touch. LGTM.

…disable (#154)
Re-scoped from the now-removed CustomFieldValue(Payload).swift to the
remaining hand-written disable in FieldValue+Codable.swift. Extracted
helpers so the cyclomatic_complexity swiftlint disable can be removed;
behavior and serialization output unchanged. Generated Operations.*.Output
disables are out of scope.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdionforce-pushed the refactor/cyclomatic-complexity-encoding branch from da921b1 to f28213bCompareJuly 1, 2026 00:48
The #154 refactor split encodeValue into encodeScalar/encodeComplex but
added no tests; the only encode-path test covered .string. Add a
parameterized encode→decode round-trip over the scalar and complex cases
(driving the encodeScalar→encodeComplex delegation), plus explicit
encode-shape assertions for .date (milliseconds) and .bytes (string
payload), which do not round-trip by design.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdion marked this pull request as ready for review July 1, 2026 15:03
@leogdion
leogdion merged commit e2619d0 into v1.0.0-beta.3Jul 1, 2026
70 of 71 checks passed
@leogdion
leogdion deleted the refactor/cyclomatic-complexity-encoding branch July 1, 2026 15:16
@claudeclaudeBot mentioned this pull request Jul 1, 2026
@claudeclaudeBot mentioned this pull request Aug 20, 2026
Sign up for freeto 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

@leogdion
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154) - #414

Merged
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding
Jul 1, 2026
Merged

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154)#414
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding

Conversation

@leogdion

Copy link
Copy Markdown
Member

Re-scope note

Issue #154 cited CustomFieldValue.swift / CustomFieldValuePayload.swiftthose files no longer exist on v1.0.0-beta.3. The remaining actionable hand-written // swiftlint:disable:next cyclomatic_complexity was in FieldValue+Codable.swift (encodeValue(to:)). The other disables live in generatedOperations.*.Output.swift and are out of scope.

What

Split encodeValue(to:) into a thin dispatcher + encodeScalar/encodeComplex helpers so each stays under the complexity threshold and the disable is removed. Behavior, public API, and serialization output unchanged (date→ms math preserved).

Closes#154.

Verification

swift build, full swift test (538 pass), swiftlint --strict clean with the disable removed.

@coderabbitai

coderabbitaiBot commented Jun 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3bc550da-9ce0-438b-b9a8-a07362de7942

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/cyclomatic-complexity-encoding

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.

@codecov

codecovBot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.26%. Comparing base (7791b09) to head (d9e65bb).

Files with missing linesPatch %Lines
...istKit/Models/FieldValues/FieldValue+Codable.swift50.00%10 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## v1.0.0-beta.3 #414 +/- ##
=================================================
+ Coverage 73.85% 74.26% +0.40% 
=================================================
Files 156 156 Lines 3698 3718 +20 =================================================
+ Hits 2731 2761 +30 + Misses 967 957 -10 
FlagCoverage Δ
mistdemo-spm-macos11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-jammy11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-noble11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.3-jammy11.51% <50.00%> (+0.31%)⬆️
mistdemo-swift-6.3-noble11.40% <50.00%> (+0.20%)⬆️
spm72.24% <50.00%> (+0.66%)⬆️
swift-6.1-jammy72.10% <50.00%> (+0.50%)⬆️
swift-6.1-noble72.42% <50.00%> (+0.77%)⬆️
swift-6.2-jammy72.12% <50.00%> (+0.52%)⬆️
swift-6.2-noble72.12% <50.00%> (+0.28%)⬆️
swift-6.3-jammy72.07% <50.00%> (+0.50%)⬆️
swift-6.3-noble72.04% <50.00%> (+0.50%)⬆️

Flags with carried forward coverage won't be shown. 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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@claude

claudeBot commented Jun 30, 2026

Copy link
Copy Markdown

Code Review

Summary: Splits encodeValue(to:) into encodeScalar + encodeComplex to bring cyclomatic complexity under the SwiftLint threshold, removing the // swiftlint:disable:next comment.


Strengths

  • Behaviour is identical — the date→ms math, byte encoding, and all other cases are preserved exactly.
  • The defensive default: throw EncodingError.invalidValue(...) in encodeComplex is exactly the right safety net: if a new FieldValue case is added in the future and the author forgets to route it, the failure is loud and immediate rather than a silent no-op.
  • encodeValue now reads as a clean two-step dispatcher, which is easier to scan at a glance.

Minor observations

Bool return from encodeScalar

The Bool signal ("did I handle it?") works, but the pattern is slightly implicit for a new reader. A name like encodeIfScalar (returning Bool) or restructuring so encodeScalar handles only its own cases and encodeComplex is always called in the else-branch would be equally clear alternatives. Not a change request — just noting the tradeoff.

Comment on encodeScalar explains WHAT, not WHY

/// Encode the scalar cases (string, bytes, int64, double, date).
///
/// - Returns: `true` when `self` was a scalar case…

Per the project's CLAUDE.md, comments should explain the non-obvious WHY. The doc comment's Returns: note is useful because the Bool return is surprising at the call site, so this is borderline acceptable. But the first line is redundant with the function name and the switch body.

default: return false in encodeScalar

This is correct, but since FieldValue is likely not @frozen, the compiler won't warn if a new case is added without handling it here. The fail-loud throw in encodeComplex catches this at runtime; adding an #if DEBUG assertion or a comment explaining why encodeComplex is the backstop would make the intent explicit.


Overall: Minimal, correct, and closes the lint issue without any behavior change. The defensive default path in encodeComplex is a nice touch. LGTM.

…disable (#154)
Re-scoped from the now-removed CustomFieldValue(Payload).swift to the
remaining hand-written disable in FieldValue+Codable.swift. Extracted
helpers so the cyclomatic_complexity swiftlint disable can be removed;
behavior and serialization output unchanged. Generated Operations.*.Output
disables are out of scope.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdionforce-pushed the refactor/cyclomatic-complexity-encoding branch from da921b1 to f28213bCompareJuly 1, 2026 00:48
The #154 refactor split encodeValue into encodeScalar/encodeComplex but
added no tests; the only encode-path test covered .string. Add a
parameterized encode→decode round-trip over the scalar and complex cases
(driving the encodeScalar→encodeComplex delegation), plus explicit
encode-shape assertions for .date (milliseconds) and .bytes (string
payload), which do not round-trip by design.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdion marked this pull request as ready for review July 1, 2026 15:03
@leogdion
leogdion merged commit e2619d0 into v1.0.0-beta.3Jul 1, 2026
70 of 71 checks passed
@leogdion
leogdion deleted the refactor/cyclomatic-complexity-encoding branch July 1, 2026 15:16
@claudeclaudeBot mentioned this pull request Jul 1, 2026
@claudeclaudeBot mentioned this pull request Aug 20, 2026
Sign up for freeto 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

@leogdion
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154) - #414

Merged
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding
Jul 1, 2026
Merged

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154)#414
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding

Conversation

@leogdion

Copy link
Copy Markdown
Member

Re-scope note

Issue #154 cited CustomFieldValue.swift / CustomFieldValuePayload.swiftthose files no longer exist on v1.0.0-beta.3. The remaining actionable hand-written // swiftlint:disable:next cyclomatic_complexity was in FieldValue+Codable.swift (encodeValue(to:)). The other disables live in generatedOperations.*.Output.swift and are out of scope.

What

Split encodeValue(to:) into a thin dispatcher + encodeScalar/encodeComplex helpers so each stays under the complexity threshold and the disable is removed. Behavior, public API, and serialization output unchanged (date→ms math preserved).

Closes#154.

Verification

swift build, full swift test (538 pass), swiftlint --strict clean with the disable removed.

@coderabbitai

coderabbitaiBot commented Jun 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3bc550da-9ce0-438b-b9a8-a07362de7942

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/cyclomatic-complexity-encoding

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.

@codecov

codecovBot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.26%. Comparing base (7791b09) to head (d9e65bb).

Files with missing linesPatch %Lines
...istKit/Models/FieldValues/FieldValue+Codable.swift50.00%10 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## v1.0.0-beta.3 #414 +/- ##
=================================================
+ Coverage 73.85% 74.26% +0.40% 
=================================================
Files 156 156 Lines 3698 3718 +20 =================================================
+ Hits 2731 2761 +30 + Misses 967 957 -10 
FlagCoverage Δ
mistdemo-spm-macos11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-jammy11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-noble11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.3-jammy11.51% <50.00%> (+0.31%)⬆️
mistdemo-swift-6.3-noble11.40% <50.00%> (+0.20%)⬆️
spm72.24% <50.00%> (+0.66%)⬆️
swift-6.1-jammy72.10% <50.00%> (+0.50%)⬆️
swift-6.1-noble72.42% <50.00%> (+0.77%)⬆️
swift-6.2-jammy72.12% <50.00%> (+0.52%)⬆️
swift-6.2-noble72.12% <50.00%> (+0.28%)⬆️
swift-6.3-jammy72.07% <50.00%> (+0.50%)⬆️
swift-6.3-noble72.04% <50.00%> (+0.50%)⬆️

Flags with carried forward coverage won't be shown. 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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@claude

claudeBot commented Jun 30, 2026

Copy link
Copy Markdown

Code Review

Summary: Splits encodeValue(to:) into encodeScalar + encodeComplex to bring cyclomatic complexity under the SwiftLint threshold, removing the // swiftlint:disable:next comment.


Strengths

  • Behaviour is identical — the date→ms math, byte encoding, and all other cases are preserved exactly.
  • The defensive default: throw EncodingError.invalidValue(...) in encodeComplex is exactly the right safety net: if a new FieldValue case is added in the future and the author forgets to route it, the failure is loud and immediate rather than a silent no-op.
  • encodeValue now reads as a clean two-step dispatcher, which is easier to scan at a glance.

Minor observations

Bool return from encodeScalar

The Bool signal ("did I handle it?") works, but the pattern is slightly implicit for a new reader. A name like encodeIfScalar (returning Bool) or restructuring so encodeScalar handles only its own cases and encodeComplex is always called in the else-branch would be equally clear alternatives. Not a change request — just noting the tradeoff.

Comment on encodeScalar explains WHAT, not WHY

/// Encode the scalar cases (string, bytes, int64, double, date).
///
/// - Returns: `true` when `self` was a scalar case…

Per the project's CLAUDE.md, comments should explain the non-obvious WHY. The doc comment's Returns: note is useful because the Bool return is surprising at the call site, so this is borderline acceptable. But the first line is redundant with the function name and the switch body.

default: return false in encodeScalar

This is correct, but since FieldValue is likely not @frozen, the compiler won't warn if a new case is added without handling it here. The fail-loud throw in encodeComplex catches this at runtime; adding an #if DEBUG assertion or a comment explaining why encodeComplex is the backstop would make the intent explicit.


Overall: Minimal, correct, and closes the lint issue without any behavior change. The defensive default path in encodeComplex is a nice touch. LGTM.

…disable (#154)
Re-scoped from the now-removed CustomFieldValue(Payload).swift to the
remaining hand-written disable in FieldValue+Codable.swift. Extracted
helpers so the cyclomatic_complexity swiftlint disable can be removed;
behavior and serialization output unchanged. Generated Operations.*.Output
disables are out of scope.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdionforce-pushed the refactor/cyclomatic-complexity-encoding branch from da921b1 to f28213bCompareJuly 1, 2026 00:48
The #154 refactor split encodeValue into encodeScalar/encodeComplex but
added no tests; the only encode-path test covered .string. Add a
parameterized encode→decode round-trip over the scalar and complex cases
(driving the encodeScalar→encodeComplex delegation), plus explicit
encode-shape assertions for .date (milliseconds) and .bytes (string
payload), which do not round-trip by design.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdion marked this pull request as ready for review July 1, 2026 15:03
@leogdion
leogdion merged commit e2619d0 into v1.0.0-beta.3Jul 1, 2026
70 of 71 checks passed
@leogdion
leogdion deleted the refactor/cyclomatic-complexity-encoding branch July 1, 2026 15:16
@claudeclaudeBot mentioned this pull request Jul 1, 2026
@claudeclaudeBot mentioned this pull request Aug 20, 2026
Sign up for freeto 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

@leogdion
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154) - #414

Merged
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding
Jul 1, 2026
Merged

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154)#414
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding

Conversation

@leogdion

Copy link
Copy Markdown
Member

Re-scope note

Issue #154 cited CustomFieldValue.swift / CustomFieldValuePayload.swiftthose files no longer exist on v1.0.0-beta.3. The remaining actionable hand-written // swiftlint:disable:next cyclomatic_complexity was in FieldValue+Codable.swift (encodeValue(to:)). The other disables live in generatedOperations.*.Output.swift and are out of scope.

What

Split encodeValue(to:) into a thin dispatcher + encodeScalar/encodeComplex helpers so each stays under the complexity threshold and the disable is removed. Behavior, public API, and serialization output unchanged (date→ms math preserved).

Closes#154.

Verification

swift build, full swift test (538 pass), swiftlint --strict clean with the disable removed.

@coderabbitai

coderabbitaiBot commented Jun 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3bc550da-9ce0-438b-b9a8-a07362de7942

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/cyclomatic-complexity-encoding

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.

@codecov

codecovBot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.26%. Comparing base (7791b09) to head (d9e65bb).

Files with missing linesPatch %Lines
...istKit/Models/FieldValues/FieldValue+Codable.swift50.00%10 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## v1.0.0-beta.3 #414 +/- ##
=================================================
+ Coverage 73.85% 74.26% +0.40% 
=================================================
Files 156 156 Lines 3698 3718 +20 =================================================
+ Hits 2731 2761 +30 + Misses 967 957 -10 
FlagCoverage Δ
mistdemo-spm-macos11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-jammy11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-noble11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.3-jammy11.51% <50.00%> (+0.31%)⬆️
mistdemo-swift-6.3-noble11.40% <50.00%> (+0.20%)⬆️
spm72.24% <50.00%> (+0.66%)⬆️
swift-6.1-jammy72.10% <50.00%> (+0.50%)⬆️
swift-6.1-noble72.42% <50.00%> (+0.77%)⬆️
swift-6.2-jammy72.12% <50.00%> (+0.52%)⬆️
swift-6.2-noble72.12% <50.00%> (+0.28%)⬆️
swift-6.3-jammy72.07% <50.00%> (+0.50%)⬆️
swift-6.3-noble72.04% <50.00%> (+0.50%)⬆️

Flags with carried forward coverage won't be shown. 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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@claude

claudeBot commented Jun 30, 2026

Copy link
Copy Markdown

Code Review

Summary: Splits encodeValue(to:) into encodeScalar + encodeComplex to bring cyclomatic complexity under the SwiftLint threshold, removing the // swiftlint:disable:next comment.


Strengths

  • Behaviour is identical — the date→ms math, byte encoding, and all other cases are preserved exactly.
  • The defensive default: throw EncodingError.invalidValue(...) in encodeComplex is exactly the right safety net: if a new FieldValue case is added in the future and the author forgets to route it, the failure is loud and immediate rather than a silent no-op.
  • encodeValue now reads as a clean two-step dispatcher, which is easier to scan at a glance.

Minor observations

Bool return from encodeScalar

The Bool signal ("did I handle it?") works, but the pattern is slightly implicit for a new reader. A name like encodeIfScalar (returning Bool) or restructuring so encodeScalar handles only its own cases and encodeComplex is always called in the else-branch would be equally clear alternatives. Not a change request — just noting the tradeoff.

Comment on encodeScalar explains WHAT, not WHY

/// Encode the scalar cases (string, bytes, int64, double, date).
///
/// - Returns: `true` when `self` was a scalar case…

Per the project's CLAUDE.md, comments should explain the non-obvious WHY. The doc comment's Returns: note is useful because the Bool return is surprising at the call site, so this is borderline acceptable. But the first line is redundant with the function name and the switch body.

default: return false in encodeScalar

This is correct, but since FieldValue is likely not @frozen, the compiler won't warn if a new case is added without handling it here. The fail-loud throw in encodeComplex catches this at runtime; adding an #if DEBUG assertion or a comment explaining why encodeComplex is the backstop would make the intent explicit.


Overall: Minimal, correct, and closes the lint issue without any behavior change. The defensive default path in encodeComplex is a nice touch. LGTM.

…disable (#154)
Re-scoped from the now-removed CustomFieldValue(Payload).swift to the
remaining hand-written disable in FieldValue+Codable.swift. Extracted
helpers so the cyclomatic_complexity swiftlint disable can be removed;
behavior and serialization output unchanged. Generated Operations.*.Output
disables are out of scope.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdionforce-pushed the refactor/cyclomatic-complexity-encoding branch from da921b1 to f28213bCompareJuly 1, 2026 00:48
The #154 refactor split encodeValue into encodeScalar/encodeComplex but
added no tests; the only encode-path test covered .string. Add a
parameterized encode→decode round-trip over the scalar and complex cases
(driving the encodeScalar→encodeComplex delegation), plus explicit
encode-shape assertions for .date (milliseconds) and .bytes (string
payload), which do not round-trip by design.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdion marked this pull request as ready for review July 1, 2026 15:03
@leogdion
leogdion merged commit e2619d0 into v1.0.0-beta.3Jul 1, 2026
70 of 71 checks passed
@leogdion
leogdion deleted the refactor/cyclomatic-complexity-encoding branch July 1, 2026 15:16
@claudeclaudeBot mentioned this pull request Jul 1, 2026
@claudeclaudeBot mentioned this pull request Aug 20, 2026
Sign up for freeto 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

@leogdion
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154) - #414

Merged
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding
Jul 1, 2026
Merged

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154)#414
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding

Conversation

@leogdion

Copy link
Copy Markdown
Member

Re-scope note

Issue #154 cited CustomFieldValue.swift / CustomFieldValuePayload.swiftthose files no longer exist on v1.0.0-beta.3. The remaining actionable hand-written // swiftlint:disable:next cyclomatic_complexity was in FieldValue+Codable.swift (encodeValue(to:)). The other disables live in generatedOperations.*.Output.swift and are out of scope.

What

Split encodeValue(to:) into a thin dispatcher + encodeScalar/encodeComplex helpers so each stays under the complexity threshold and the disable is removed. Behavior, public API, and serialization output unchanged (date→ms math preserved).

Closes#154.

Verification

swift build, full swift test (538 pass), swiftlint --strict clean with the disable removed.

@coderabbitai

coderabbitaiBot commented Jun 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3bc550da-9ce0-438b-b9a8-a07362de7942

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/cyclomatic-complexity-encoding

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.

@codecov

codecovBot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.26%. Comparing base (7791b09) to head (d9e65bb).

Files with missing linesPatch %Lines
...istKit/Models/FieldValues/FieldValue+Codable.swift50.00%10 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## v1.0.0-beta.3 #414 +/- ##
=================================================
+ Coverage 73.85% 74.26% +0.40% 
=================================================
Files 156 156 Lines 3698 3718 +20 =================================================
+ Hits 2731 2761 +30 + Misses 967 957 -10 
FlagCoverage Δ
mistdemo-spm-macos11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-jammy11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-noble11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.3-jammy11.51% <50.00%> (+0.31%)⬆️
mistdemo-swift-6.3-noble11.40% <50.00%> (+0.20%)⬆️
spm72.24% <50.00%> (+0.66%)⬆️
swift-6.1-jammy72.10% <50.00%> (+0.50%)⬆️
swift-6.1-noble72.42% <50.00%> (+0.77%)⬆️
swift-6.2-jammy72.12% <50.00%> (+0.52%)⬆️
swift-6.2-noble72.12% <50.00%> (+0.28%)⬆️
swift-6.3-jammy72.07% <50.00%> (+0.50%)⬆️
swift-6.3-noble72.04% <50.00%> (+0.50%)⬆️

Flags with carried forward coverage won't be shown. 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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@claude

claudeBot commented Jun 30, 2026

Copy link
Copy Markdown

Code Review

Summary: Splits encodeValue(to:) into encodeScalar + encodeComplex to bring cyclomatic complexity under the SwiftLint threshold, removing the // swiftlint:disable:next comment.


Strengths

  • Behaviour is identical — the date→ms math, byte encoding, and all other cases are preserved exactly.
  • The defensive default: throw EncodingError.invalidValue(...) in encodeComplex is exactly the right safety net: if a new FieldValue case is added in the future and the author forgets to route it, the failure is loud and immediate rather than a silent no-op.
  • encodeValue now reads as a clean two-step dispatcher, which is easier to scan at a glance.

Minor observations

Bool return from encodeScalar

The Bool signal ("did I handle it?") works, but the pattern is slightly implicit for a new reader. A name like encodeIfScalar (returning Bool) or restructuring so encodeScalar handles only its own cases and encodeComplex is always called in the else-branch would be equally clear alternatives. Not a change request — just noting the tradeoff.

Comment on encodeScalar explains WHAT, not WHY

/// Encode the scalar cases (string, bytes, int64, double, date).
///
/// - Returns: `true` when `self` was a scalar case…

Per the project's CLAUDE.md, comments should explain the non-obvious WHY. The doc comment's Returns: note is useful because the Bool return is surprising at the call site, so this is borderline acceptable. But the first line is redundant with the function name and the switch body.

default: return false in encodeScalar

This is correct, but since FieldValue is likely not @frozen, the compiler won't warn if a new case is added without handling it here. The fail-loud throw in encodeComplex catches this at runtime; adding an #if DEBUG assertion or a comment explaining why encodeComplex is the backstop would make the intent explicit.


Overall: Minimal, correct, and closes the lint issue without any behavior change. The defensive default path in encodeComplex is a nice touch. LGTM.

…disable (#154)
Re-scoped from the now-removed CustomFieldValue(Payload).swift to the
remaining hand-written disable in FieldValue+Codable.swift. Extracted
helpers so the cyclomatic_complexity swiftlint disable can be removed;
behavior and serialization output unchanged. Generated Operations.*.Output
disables are out of scope.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdionforce-pushed the refactor/cyclomatic-complexity-encoding branch from da921b1 to f28213bCompareJuly 1, 2026 00:48
The #154 refactor split encodeValue into encodeScalar/encodeComplex but
added no tests; the only encode-path test covered .string. Add a
parameterized encode→decode round-trip over the scalar and complex cases
(driving the encodeScalar→encodeComplex delegation), plus explicit
encode-shape assertions for .date (milliseconds) and .bytes (string
payload), which do not round-trip by design.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdion marked this pull request as ready for review July 1, 2026 15:03
@leogdion
leogdion merged commit e2619d0 into v1.0.0-beta.3Jul 1, 2026
70 of 71 checks passed
@leogdion
leogdion deleted the refactor/cyclomatic-complexity-encoding branch July 1, 2026 15:16
@claudeclaudeBot mentioned this pull request Jul 1, 2026
@claudeclaudeBot mentioned this pull request Aug 20, 2026
Sign up for freeto 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

@leogdion
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154) - #414

Merged
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding
Jul 1, 2026
Merged

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154)#414
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding

Conversation

@leogdion

Copy link
Copy Markdown
Member

Re-scope note

Issue #154 cited CustomFieldValue.swift / CustomFieldValuePayload.swiftthose files no longer exist on v1.0.0-beta.3. The remaining actionable hand-written // swiftlint:disable:next cyclomatic_complexity was in FieldValue+Codable.swift (encodeValue(to:)). The other disables live in generatedOperations.*.Output.swift and are out of scope.

What

Split encodeValue(to:) into a thin dispatcher + encodeScalar/encodeComplex helpers so each stays under the complexity threshold and the disable is removed. Behavior, public API, and serialization output unchanged (date→ms math preserved).

Closes#154.

Verification

swift build, full swift test (538 pass), swiftlint --strict clean with the disable removed.

@coderabbitai

coderabbitaiBot commented Jun 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3bc550da-9ce0-438b-b9a8-a07362de7942

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/cyclomatic-complexity-encoding

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.

@codecov

codecovBot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.26%. Comparing base (7791b09) to head (d9e65bb).

Files with missing linesPatch %Lines
...istKit/Models/FieldValues/FieldValue+Codable.swift50.00%10 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## v1.0.0-beta.3 #414 +/- ##
=================================================
+ Coverage 73.85% 74.26% +0.40% 
=================================================
Files 156 156 Lines 3698 3718 +20 =================================================
+ Hits 2731 2761 +30 + Misses 967 957 -10 
FlagCoverage Δ
mistdemo-spm-macos11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-jammy11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-noble11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.3-jammy11.51% <50.00%> (+0.31%)⬆️
mistdemo-swift-6.3-noble11.40% <50.00%> (+0.20%)⬆️
spm72.24% <50.00%> (+0.66%)⬆️
swift-6.1-jammy72.10% <50.00%> (+0.50%)⬆️
swift-6.1-noble72.42% <50.00%> (+0.77%)⬆️
swift-6.2-jammy72.12% <50.00%> (+0.52%)⬆️
swift-6.2-noble72.12% <50.00%> (+0.28%)⬆️
swift-6.3-jammy72.07% <50.00%> (+0.50%)⬆️
swift-6.3-noble72.04% <50.00%> (+0.50%)⬆️

Flags with carried forward coverage won't be shown. 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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@claude

claudeBot commented Jun 30, 2026

Copy link
Copy Markdown

Code Review

Summary: Splits encodeValue(to:) into encodeScalar + encodeComplex to bring cyclomatic complexity under the SwiftLint threshold, removing the // swiftlint:disable:next comment.


Strengths

  • Behaviour is identical — the date→ms math, byte encoding, and all other cases are preserved exactly.
  • The defensive default: throw EncodingError.invalidValue(...) in encodeComplex is exactly the right safety net: if a new FieldValue case is added in the future and the author forgets to route it, the failure is loud and immediate rather than a silent no-op.
  • encodeValue now reads as a clean two-step dispatcher, which is easier to scan at a glance.

Minor observations

Bool return from encodeScalar

The Bool signal ("did I handle it?") works, but the pattern is slightly implicit for a new reader. A name like encodeIfScalar (returning Bool) or restructuring so encodeScalar handles only its own cases and encodeComplex is always called in the else-branch would be equally clear alternatives. Not a change request — just noting the tradeoff.

Comment on encodeScalar explains WHAT, not WHY

/// Encode the scalar cases (string, bytes, int64, double, date).
///
/// - Returns: `true` when `self` was a scalar case…

Per the project's CLAUDE.md, comments should explain the non-obvious WHY. The doc comment's Returns: note is useful because the Bool return is surprising at the call site, so this is borderline acceptable. But the first line is redundant with the function name and the switch body.

default: return false in encodeScalar

This is correct, but since FieldValue is likely not @frozen, the compiler won't warn if a new case is added without handling it here. The fail-loud throw in encodeComplex catches this at runtime; adding an #if DEBUG assertion or a comment explaining why encodeComplex is the backstop would make the intent explicit.


Overall: Minimal, correct, and closes the lint issue without any behavior change. The defensive default path in encodeComplex is a nice touch. LGTM.

…disable (#154)
Re-scoped from the now-removed CustomFieldValue(Payload).swift to the
remaining hand-written disable in FieldValue+Codable.swift. Extracted
helpers so the cyclomatic_complexity swiftlint disable can be removed;
behavior and serialization output unchanged. Generated Operations.*.Output
disables are out of scope.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdionforce-pushed the refactor/cyclomatic-complexity-encoding branch from da921b1 to f28213bCompareJuly 1, 2026 00:48
The #154 refactor split encodeValue into encodeScalar/encodeComplex but
added no tests; the only encode-path test covered .string. Add a
parameterized encode→decode round-trip over the scalar and complex cases
(driving the encodeScalar→encodeComplex delegation), plus explicit
encode-shape assertions for .date (milliseconds) and .bytes (string
payload), which do not round-trip by design.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdion marked this pull request as ready for review July 1, 2026 15:03
@leogdion
leogdion merged commit e2619d0 into v1.0.0-beta.3Jul 1, 2026
70 of 71 checks passed
@leogdion
leogdion deleted the refactor/cyclomatic-complexity-encoding branch July 1, 2026 15:16
@claudeclaudeBot mentioned this pull request Jul 1, 2026
@claudeclaudeBot mentioned this pull request Aug 20, 2026
Sign up for freeto 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

@leogdion
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154) - #414

Merged
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding
Jul 1, 2026
Merged

Reduce cyclomatic complexity in FieldValue+Codable, drop lint disable (#154)#414
leogdion merged 2 commits into
v1.0.0-beta.3from
refactor/cyclomatic-complexity-encoding

Conversation

@leogdion

Copy link
Copy Markdown
Member

Re-scope note

Issue #154 cited CustomFieldValue.swift / CustomFieldValuePayload.swiftthose files no longer exist on v1.0.0-beta.3. The remaining actionable hand-written // swiftlint:disable:next cyclomatic_complexity was in FieldValue+Codable.swift (encodeValue(to:)). The other disables live in generatedOperations.*.Output.swift and are out of scope.

What

Split encodeValue(to:) into a thin dispatcher + encodeScalar/encodeComplex helpers so each stays under the complexity threshold and the disable is removed. Behavior, public API, and serialization output unchanged (date→ms math preserved).

Closes#154.

Verification

swift build, full swift test (538 pass), swiftlint --strict clean with the disable removed.

@coderabbitai

coderabbitaiBot commented Jun 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3bc550da-9ce0-438b-b9a8-a07362de7942

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/cyclomatic-complexity-encoding

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.

@codecov

codecovBot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.26%. Comparing base (7791b09) to head (d9e65bb).

Files with missing linesPatch %Lines
...istKit/Models/FieldValues/FieldValue+Codable.swift50.00%10 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## v1.0.0-beta.3 #414 +/- ##
=================================================
+ Coverage 73.85% 74.26% +0.40% 
=================================================
Files 156 156 Lines 3698 3718 +20 =================================================
+ Hits 2731 2761 +30 + Misses 967 957 -10 
FlagCoverage Δ
mistdemo-spm-macos11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-jammy11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.2-noble11.40% <50.00%> (+0.20%)⬆️
mistdemo-swift-6.3-jammy11.51% <50.00%> (+0.31%)⬆️
mistdemo-swift-6.3-noble11.40% <50.00%> (+0.20%)⬆️
spm72.24% <50.00%> (+0.66%)⬆️
swift-6.1-jammy72.10% <50.00%> (+0.50%)⬆️
swift-6.1-noble72.42% <50.00%> (+0.77%)⬆️
swift-6.2-jammy72.12% <50.00%> (+0.52%)⬆️
swift-6.2-noble72.12% <50.00%> (+0.28%)⬆️
swift-6.3-jammy72.07% <50.00%> (+0.50%)⬆️
swift-6.3-noble72.04% <50.00%> (+0.50%)⬆️

Flags with carried forward coverage won't be shown. 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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@claude

claudeBot commented Jun 30, 2026

Copy link
Copy Markdown

Code Review

Summary: Splits encodeValue(to:) into encodeScalar + encodeComplex to bring cyclomatic complexity under the SwiftLint threshold, removing the // swiftlint:disable:next comment.


Strengths

  • Behaviour is identical — the date→ms math, byte encoding, and all other cases are preserved exactly.
  • The defensive default: throw EncodingError.invalidValue(...) in encodeComplex is exactly the right safety net: if a new FieldValue case is added in the future and the author forgets to route it, the failure is loud and immediate rather than a silent no-op.
  • encodeValue now reads as a clean two-step dispatcher, which is easier to scan at a glance.

Minor observations

Bool return from encodeScalar

The Bool signal ("did I handle it?") works, but the pattern is slightly implicit for a new reader. A name like encodeIfScalar (returning Bool) or restructuring so encodeScalar handles only its own cases and encodeComplex is always called in the else-branch would be equally clear alternatives. Not a change request — just noting the tradeoff.

Comment on encodeScalar explains WHAT, not WHY

/// Encode the scalar cases (string, bytes, int64, double, date).
///
/// - Returns: `true` when `self` was a scalar case…

Per the project's CLAUDE.md, comments should explain the non-obvious WHY. The doc comment's Returns: note is useful because the Bool return is surprising at the call site, so this is borderline acceptable. But the first line is redundant with the function name and the switch body.

default: return false in encodeScalar

This is correct, but since FieldValue is likely not @frozen, the compiler won't warn if a new case is added without handling it here. The fail-loud throw in encodeComplex catches this at runtime; adding an #if DEBUG assertion or a comment explaining why encodeComplex is the backstop would make the intent explicit.


Overall: Minimal, correct, and closes the lint issue without any behavior change. The defensive default path in encodeComplex is a nice touch. LGTM.

…disable (#154)
Re-scoped from the now-removed CustomFieldValue(Payload).swift to the
remaining hand-written disable in FieldValue+Codable.swift. Extracted
helpers so the cyclomatic_complexity swiftlint disable can be removed;
behavior and serialization output unchanged. Generated Operations.*.Output
disables are out of scope.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdionforce-pushed the refactor/cyclomatic-complexity-encoding branch from da921b1 to f28213bCompareJuly 1, 2026 00:48
The #154 refactor split encodeValue into encodeScalar/encodeComplex but
added no tests; the only encode-path test covered .string. Add a
parameterized encode→decode round-trip over the scalar and complex cases
(driving the encodeScalar→encodeComplex delegation), plus explicit
encode-shape assertions for .date (milliseconds) and .bytes (string
payload), which do not round-trip by design.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@leogdion
leogdion marked this pull request as ready for review July 1, 2026 15:03
@leogdion
leogdion merged commit e2619d0 into v1.0.0-beta.3Jul 1, 2026
70 of 71 checks passed
@leogdion
leogdion deleted the refactor/cyclomatic-complexity-encoding branch July 1, 2026 15:16
@claudeclaudeBot mentioned this pull request Jul 1, 2026
@claudeclaudeBot mentioned this pull request Aug 20, 2026
Sign up for freeto 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

@leogdion