Upgrades bottom nav bar Upgrade Bottom Navigation Bar - #61
Merged
Conversation
…ation to F-Droid repository data
…hain - compileSdk + targetSdk: 36 → 37 (Android 17 / API 37) - Removed compileSdkExtension (not needed for base API 37) - Java source/target compatibility: VERSION_11 → VERSION_17 - Kotlin jvmTarget: 11 → 17 - Gradle wrapper: 8.12 → 8.14.1 - AGP: 8.9.1 → 8.11.1 - Kotlin Gradle Plugin: 2.1.0 → 2.2.20 - Enable android.builtInKotlin=true + android.newDsl=true - Remove explicit id(kotlin-android) plugin (now injected by Flutter)
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe pull request updates Android release tooling and CI workflows, replaces the home navigation bar with a floating implementation, changes workout completion navigation, and adds service, storage, model, and widget-flow tests. ChangesAndroid release and CI
Floating navigation
Application behavior and validation
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
coderabbitai
Bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/release.yml:
- Line 140: Update the release workflow step containing the Flutter APK build to
retain both Dart symbols from build/app/outputs/symbols and the R8 mapping file
at android/app/build/outputs/mapping/release/mapping.txt. Add a private artifact
upload with retention configured so these deobfuscation files are preserved
alongside the published APKs.
- Around line 127-130: Ensure release signing fails when any credential is
missing: in .github/workflows/release.yml lines 127-130, validate
KEYSTORE_BASE64, KEY_STORE_PASSWORD, KEY_ALIAS, and KEY_PASSWORD before
building; in workout-logger/android/app/build.gradle.kts lines 44-48, treat
blank environment values as absent before falling back to key.properties,
preventing incomplete credentials from selecting debug signing.
- Line 25: Pin every listed GitHub Action to a reviewed immutable commit SHA
instead of a mutable tag or branch. Update actions/checkout, actions/setup-java,
gradle/actions/setup-gradle, subosito/flutter-action, actions/upload-artifact,
softprops/action-gh-release, and codecov/codecov-action at all affected sites:
.github/workflows/release.yml lines 25-25, 31-31, 37-37, and 164-164, plus
.github/workflows/test.yml line 25-25.
In @.github/workflows/test.yml:
- Line 64: Remove the trailing blank line at the end of the workflow file so the
YAML lint check passes.
- Around line 37-40: Update the dependency installation step in the workflow to
run flutter pub get on every checkout by removing the CACHE-HIT condition from
the step using the flutter-action output. Preserve its working-directory and
command.
In `@workout-logger/lib/screens/widgets/floating_nav_bar.dart`:
- Around line 515-535: Update the chip decoration in the floating navigation bar
to use FloatingNavBarTheme.defaultChipBorderOpacity for the border alpha and
defaultChipShadowOpacity for the shadow alpha instead of the hardcoded 0.28 and
0.18 values. Preserve the existing colorP scaling and conditional rendering.
- Around line 584-602: Wrap the label Text in the showLabels branch of the
floating navigation bar’s chip content with Flexible, preserving its existing
opacity, styling, single-line, and clipping behavior so long labels remain
within the fixed-width Container.
🪄 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 Plus
Run ID: 5cc2e9ba-cbed-47fe-9db7-2bf5de8b69a1
⛔ Files ignored due to path filters (1)
workout-logger/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
.github/workflows/release.yml.github/workflows/test.ymlREADME.mdworkout-logger/.gitignoreworkout-logger/android/app/build.gradle.ktsworkout-logger/android/app/proguard-rules.proworkout-logger/android/gradle.propertiesworkout-logger/android/gradle/wrapper/gradle-wrapper.propertiesworkout-logger/android/key.properties.exampleworkout-logger/android/settings.gradle.ktsworkout-logger/fastlane/metadata/android/en-US/full_description.txtworkout-logger/lib/screens/home_screen.dartworkout-logger/lib/screens/widgets/floating_nav_bar.dartworkout-logger/lib/screens/widgets/rf_widgets.dartworkout-logger/pubspec.yamlworkout-logger/scripts/build_release.py
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
| uses: actions/checkout@v7 |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow pins =="
grep -nE 'uses: [^[:space:]]+@' .github/workflows/release.yml .github/workflows/test.yml 2>/dev/null || true
echo
echo "== tracked workflow files =="
git ls-files .github/workflows || trueRepository: Devasy/RepForge
Length of output: 907
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
for path in [Path(".github/workflows/release.yml"), Path(".github/workflows/test.yml")]:
print(f"\n== {path} ==")
for i, line in enumerate(path.read_text().splitlines(), 1):
m = re.search(r'uses:\s*([^\s#]+)@([^\s#]+)', line)
if m:
ref = m.group(2)
status = "SHA-like" if re.fullmatch(r'[0-9a-fA-F]{40}', ref) else "ref-like"
print(f"{i}: {m.group(1)}@{m.group(2)} :: {status}")
PYRepository: Devasy/RepForge
Length of output: 601
Pin GitHub Actions to immutable commit SHAs. Mutable tags/branches let upstream changes alter release or test workflow execution. Pin the checked-in action references to reviewed commit SHAs, including actions/checkout, actions/setup-java, gradle/actions/setup-gradle, subosito/flutter-action, actions/upload-artifact, softprops/action-gh-release, and codecov/codecov-action.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 25-25: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
📍 Affects 2 files
.github/workflows/release.yml#L25-L25(this comment).github/workflows/release.yml#L31-L31.github/workflows/release.yml#L37-L37.github/workflows/release.yml#L164-L164.github/workflows/test.yml#L25-L25
🤖 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 @.github/workflows/release.yml at line 25, Pin every listed GitHub Action to
a reviewed immutable commit SHA instead of a mutable tag or branch. Update
actions/checkout, actions/setup-java, gradle/actions/setup-gradle,
subosito/flutter-action, actions/upload-artifact, softprops/action-gh-release,
and codecov/codecov-action at all affected sites: .github/workflows/release.yml
lines 25-25, 31-31, 37-37, and 164-164, plus .github/workflows/test.yml line
25-25.
Source: Linters/SAST tools
Comment on lines
+127
to
+130
| if [ -z "${{ secrets.KEYSTORE_BASE64 }}" ]; then | ||
| echo "Error: KEYSTORE_BASE64 secret is not configured in repository secrets." | ||
| exit 1 | ||
| fi |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Fail releases when any signing credential is absent. The workflow validates only KEYSTORE_BASE64, while Gradle leaves release signing unset when any password or alias is blank and silently signs with the debug key.
.github/workflows/release.yml#L127-L130: validateKEYSTORE_BASE64,KEY_STORE_PASSWORD,KEY_ALIAS, andKEY_PASSWORDbefore building.workout-logger/android/app/build.gradle.kts#L44-L48: treat blank environment values as absent before falling back tokey.properties.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 127-127: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
📍 Affects 2 files
.github/workflows/release.yml#L127-L130(this comment)workout-logger/android/app/build.gradle.kts#L44-L48
🤖 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 @.github/workflows/release.yml around lines 127 - 130, Ensure release signing
fails when any credential is missing: in .github/workflows/release.yml lines
127-130, validate KEYSTORE_BASE64, KEY_STORE_PASSWORD, KEY_ALIAS, and
KEY_PASSWORD before building; in workout-logger/android/app/build.gradle.kts
lines 44-48, treat blank environment values as absent before falling back to
key.properties, preventing incomplete credentials from selecting debug signing.
| KEY_ALIAS: ${{ secrets.KEY_ALIAS }} | ||
| KEY_PASSWORD: ${{ secrets.KEY_PASSWORD }} | ||
| run: flutter build apk --release --split-per-abi | ||
| run: flutter build apk --release --split-per-abi --obfuscate --split-debug-info=build/app/outputs/symbols |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Retain Dart and R8 deobfuscation artifacts.
This runner is ephemeral and later steps only publish APKs, so production stack traces cannot be decoded. Upload build/app/outputs/symbols and android/app/build/outputs/mapping/release/mapping.txt as a private retained artifact.
🤖 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 @.github/workflows/release.yml at line 140, Update the release workflow step
containing the Flutter APK build to retain both Dart symbols from
build/app/outputs/symbols and the R8 mapping file at
android/app/build/outputs/mapping/release/mapping.txt. Add a private artifact
upload with retention configured so these deobfuscation files are preserved
alongside the published APKs.
Comment on lines
37
to
40
| - name: Install dependencies | ||
| if: steps.flutter-action.outputs.PUB-CACHE-HIT != 'true' | ||
| if: steps.flutter-action.outputs.CACHE-HIT != 'true' | ||
| working-directory: ./workout-logger | ||
| run: flutter pub get |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow files =="
git ls-files .github/workflows || true
echo "== test workflow excerpt =="
if [ -f .github/workflows/test.yml ]; then
nl -ba .github/workflows/test.yml | sed -n '1,140p'
fi
echo "== flutter-action/cache references =="
rg -n "flutter-action|CACHE-HIT|flutter pub get|pub get|dart_tool/package_config|flutter analyze|flutter test" .github/workflows workout-logger 2>/dev/null || true
echo "== flutter action usage context in tracked files =="
for f in $(git ls-files .github/workflows); do
echo "--- $f"
nl -ba "$f" | sed -n '1,220p'
doneRepository: Devasy/RepForge
Length of output: 299
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow excerpts =="
for f in .github/workflows/release.yml .github/workflows/test.yml; do
if [ -f "$f" ]; then
echo "--- $f"
awk '{printf "%6d %s\n", NR, $0}' "$f" | sed -n '1,220p'
fi
done
echo "== search relevant references =="
rg -n "flutter-action|CACHE-HIT|flutter pub get|pub get|dart_tool/package_config|flutter analyze|flutter test|subosito" .github/workflows workout-logger 2>/dev/null || trueRepository: Devasy/RepForge
Length of output: 11942
🌐 Web query:
subosito flutter-action v2 cache true CACHE-HIT output generated files .dart_tool/package_config.json
💡 Result:
The subosito/flutter-action action (v2) with cache: true enabled does not automatically cache the.dart_tool/package_config.json file or your project's generated files [1][2][3]. When you enable cache: true, the action uses actions/cache internally to cache two specific areas [4]: 1. The Flutter SDK installation [1][4]. 2. The Dart/Flutter pub package cache (commonly located in ~/.pub-cache) [1][4][5]. The CACHE-HIT and PUB-CACHE-HIT outputs provided by the action reflect whether an exact match was found for these specific SDK and pub caches, respectively [2][6][3]. They do not indicate the presence or validity of your project's local build artifacts like.dart_tool/package_config.json [2][7]. If you need to persist generated files (such as those in.dart_tool,.g.dart files, etc.) across workflow runs, you must explicitly add a separate actions/cache step to your workflow configuration [8]. Example of how to manually cache these files: - name: Cache generated files uses: actions/cache@v4 with: path: |.dart_tool/package_config.json /.dart_tool key: build-artifacts-${{ hashFiles('/pubspec.lock') }} [8] The CACHE-HIT output from subosito/flutter-action is solely for the action's own internal caching logic [2][9]. You should not rely on it as a proxy for the state of your application's generated build artifacts [2][7].
Citations:
- 1: https://github.com/subosito/flutter-action/blob/main/action.yaml
- 2: https://github.com/subosito/flutter-action/blob/main/README.md
- 3: https://github.com/subosito/flutter-action
- 4: How to integrate with caching subosito/flutter-action#283
- 5: Add separate pub-cache boolean flag subosito/flutter-action#377
- 6: Add cache hit outputs subosito/flutter-action#364
- 7: feat: impl cache hit outputs subosito/flutter-action#346
- 8: https://gist.github.com/sezabass/73c178512bff7589b45b820bf1bd37b2
- 9: https://github.com/marketplace/actions/flutter-action
Run flutter pub get on every checkout.
A cache hit from subosito/flutter-action restores the Flutter package cache/SDK, not the checkout’s generated .dart_tool/package_config.json; skipping flutter pub get on cache hits can make analyze or test fail.
Proposed fix
- name: Install dependencies
- if: steps.flutter-action.outputs.CACHE-HIT != 'true'
working-directory: ./workout-logger
run: flutter pub get📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Install dependencies | |
| if: steps.flutter-action.outputs.PUB-CACHE-HIT != 'true' | |
| if: steps.flutter-action.outputs.CACHE-HIT != 'true' | |
| working-directory: ./workout-logger | |
| run: flutter pub get | |
| - name: Install dependencies | |
| working-directory: ./workout-logger | |
| run: flutter pub get |
🤖 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 @.github/workflows/test.yml around lines 37 - 40, Update the dependency
installation step in the workflow to run flutter pub get on every checkout by
removing the CACHE-HIT condition from the step using the flutter-action output.
Preserve its working-directory and command.
| with: | ||
| files: workout-logger/coverage/lcov.info | ||
| token: ${{ secrets.CODECOV_TOKEN }} | ||
|
|
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the trailing blank line.
YAMLlint reports this as an error, so the workflow lint check will fail.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 64-64: too many blank lines (1 > 0)
(empty-lines)
🤖 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 @.github/workflows/test.yml at line 64, Remove the trailing blank line at the
end of the workflow file so the YAML lint check passes.
Source: Linters/SAST tools
Comment on lines
+515
to
+535
| decoration: BoxDecoration( | ||
| color: Color.lerp(Colors.transparent, widget.chipBg, colorP), | ||
| borderRadius: BorderRadius.circular(9999), | ||
| border: colorP > 0.05 | ||
| ? Border.all( | ||
| color: (t.selectedChipBorderColor ?? widget.chipContent) | ||
| .withValues(alpha: 0.28 * colorP), | ||
| width: 1.0, | ||
| ) | ||
| : null, | ||
| boxShadow: colorP > 0.05 | ||
| ? [ | ||
| BoxShadow( | ||
| color: | ||
| (t.selectedChipShadowColor ?? widget.chipContent) | ||
| .withValues(alpha: 0.18 * colorP), | ||
| blurRadius: 14, | ||
| spreadRadius: -2, | ||
| ), | ||
| ] | ||
| : null, |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Chip border/shadow opacities are hardcoded, ignoring the theme fields.
defaultChipBorderOpacity (0.25) and defaultChipShadowOpacity (0.15) are declared on FloatingNavBarTheme (Lines 209-210) but never read — the chip decoration uses literals 0.28 and 0.18 instead, so these knobs are dead. Wire them through for a consistent, configurable API.
♻️ Use theme-configured opacities
border: colorP > 0.05
? Border.all(
color: (t.selectedChipBorderColor ?? widget.chipContent)
- .withValues(alpha: 0.28 * colorP),
+ .withValues(alpha: t.defaultChipBorderOpacity * colorP),
width: 1.0,
)
: null,
boxShadow: colorP > 0.05
? [
BoxShadow(
color:
(t.selectedChipShadowColor ?? widget.chipContent)
- .withValues(alpha: 0.18 * colorP),
+ .withValues(alpha: t.defaultChipShadowOpacity * colorP),
blurRadius: 14,
spreadRadius: -2,
),
]
: null,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| decoration: BoxDecoration( | |
| color: Color.lerp(Colors.transparent, widget.chipBg, colorP), | |
| borderRadius: BorderRadius.circular(9999), | |
| border: colorP > 0.05 | |
| ? Border.all( | |
| color: (t.selectedChipBorderColor ?? widget.chipContent) | |
| .withValues(alpha: 0.28 * colorP), | |
| width: 1.0, | |
| ) | |
| : null, | |
| boxShadow: colorP > 0.05 | |
| ? [ | |
| BoxShadow( | |
| color: | |
| (t.selectedChipShadowColor ?? widget.chipContent) | |
| .withValues(alpha: 0.18 * colorP), | |
| blurRadius: 14, | |
| spreadRadius: -2, | |
| ), | |
| ] | |
| : null, | |
| decoration: BoxDecoration( | |
| color: Color.lerp(Colors.transparent, widget.chipBg, colorP), | |
| borderRadius: BorderRadius.circular(9999), | |
| border: colorP > 0.05 | |
| ? Border.all( | |
| color: (t.selectedChipBorderColor ?? widget.chipContent) | |
| .withValues(alpha: t.defaultChipBorderOpacity * colorP), | |
| width: 1.0, | |
| ) | |
| : null, | |
| boxShadow: colorP > 0.05 | |
| ? [ | |
| BoxShadow( | |
| color: | |
| (t.selectedChipShadowColor ?? widget.chipContent) | |
| .withValues(alpha: t.defaultChipShadowOpacity * colorP), | |
| blurRadius: 14, | |
| spreadRadius: -2, | |
| ), | |
| ] | |
| : null, |
🤖 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 `@workout-logger/lib/screens/widgets/floating_nav_bar.dart` around lines 515 -
535, Update the chip decoration in the floating navigation bar to use
FloatingNavBarTheme.defaultChipBorderOpacity for the border alpha and
defaultChipShadowOpacity for the shadow alpha instead of the hardcoded 0.28 and
0.18 values. Preserve the existing colorP scaling and conditional rendering.
Comment on lines
+584
to
+602
| if (t.showLabels && extraW > 1.0) ...[ | ||
| Opacity( | ||
| opacity: labelOpacity, | ||
| child: Text( | ||
| widget.item.label, | ||
| style: (t.labelStyle ?? | ||
| const TextStyle( | ||
| fontSize: 13, | ||
| fontWeight: FontWeight.w600, | ||
| letterSpacing: 0.1, | ||
| )) | ||
| .copyWith(color: widget.chipContent), | ||
| maxLines: 1, | ||
| softWrap: false, | ||
| overflow: TextOverflow.clip, | ||
| ), | ||
| ), | ||
| SizedBox(width: rightPad), | ||
| ], |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Unconstrained label can overflow the fixed-width chip.
The chip Container width is clamped using the constant labelWidth (Line 511-514), but the label Text has an intrinsic width driven by the actual string. For longer labels the Row (mainAxisSize.min) can exceed the container width and trip a RenderFlex overflow. Since this is a reusable widget, wrap the label in Flexible so it clips within the available space instead of overflowing.
🛡️ Constrain the label
- Opacity(
- opacity: labelOpacity,
- child: Text(
- widget.item.label,
- style: (t.labelStyle ??
- const TextStyle(
- fontSize: 13,
- fontWeight: FontWeight.w600,
- letterSpacing: 0.1,
- ))
- .copyWith(color: widget.chipContent),
- maxLines: 1,
- softWrap: false,
- overflow: TextOverflow.clip,
- ),
- ),
+ Flexible(
+ child: Opacity(
+ opacity: labelOpacity,
+ child: Text(
+ widget.item.label,
+ style: (t.labelStyle ??
+ const TextStyle(
+ fontSize: 13,
+ fontWeight: FontWeight.w600,
+ letterSpacing: 0.1,
+ ))
+ .copyWith(color: widget.chipContent),
+ maxLines: 1,
+ softWrap: false,
+ overflow: TextOverflow.clip,
+ ),
+ ),
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (t.showLabels && extraW > 1.0) ...[ | |
| Opacity( | |
| opacity: labelOpacity, | |
| child: Text( | |
| widget.item.label, | |
| style: (t.labelStyle ?? | |
| const TextStyle( | |
| fontSize: 13, | |
| fontWeight: FontWeight.w600, | |
| letterSpacing: 0.1, | |
| )) | |
| .copyWith(color: widget.chipContent), | |
| maxLines: 1, | |
| softWrap: false, | |
| overflow: TextOverflow.clip, | |
| ), | |
| ), | |
| SizedBox(width: rightPad), | |
| ], | |
| Flexible( | |
| child: Opacity( | |
| opacity: labelOpacity, | |
| child: Text( | |
| widget.item.label, | |
| style: (t.labelStyle ?? | |
| const TextStyle( | |
| fontSize: 13, | |
| fontWeight: FontWeight.w600, | |
| letterSpacing: 0.1, | |
| )) | |
| .copyWith(color: widget.chipContent), | |
| maxLines: 1, | |
| softWrap: false, | |
| overflow: TextOverflow.clip, | |
| ), | |
| ), | |
| ), | |
| SizedBox(width: rightPad), |
🤖 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 `@workout-logger/lib/screens/widgets/floating_nav_bar.dart` around lines 584 -
602, Wrap the label Text in the showLabels branch of the floating navigation
bar’s chip content with Flexible, preserving its existing opacity, styling,
single-line, and clipping behavior so long labels remain within the fixed-width
Container.
…ades-bottom-nav-bar
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@workout-logger/test/sleep_hr_builder_test.dart`:
- Around line 7-38: Extract the duplicated _StubHcService implementation from
workout-logger/test/sleep_hr_builder_test.dart lines 7-38 into
workout-logger/test/test_utils/stub_health_connect_service.dart, preserving its
constructor defaults and IHealthConnectService behavior. Update
workout-logger/test/sleep_hr_builder_test.dart lines 7-38 to import and use the
shared helper, and replace the duplicate definition in
workout-logger/test/userflow_history_and_session_details_test.dart lines 15-36
with the same shared helper.
In `@workout-logger/test/sleep_hr_models_test.dart`:
- Around line 24-32: Make the SleepStageStats construction assigned to stats
const, matching the existing const construction later in the test and the
literal-only arguments.
In `@workout-logger/test/userflow_history_and_session_details_test.dart`:
- Around line 124-151: The tests directly construct sub-widgets instead of
exercising their real userflow transitions. In
workout-logger/test/userflow_history_and_session_details_test.dart:124-151,
update the test around SessionDetailsSheet to pump HistoryScreen and tap the
relevant history list item; in
workout-logger/test/userflow_workout_logging_test.dart:79-129, drive
WorkoutFlowScreen through an actual rest-timer countdown and workout completion
so RestTimerView and WorkoutSummaryScreen appear through the production flow,
preserving the existing assertions.
In `@workout-logger/test/userflow_settings_and_storage_test.dart`:
- Around line 64-76: The test “Toggling weight unit in SettingsProvider persists
to storage and updates display label” only checks provider state, not rendered
UI. Update this test to render SettingsScreen with the test harness, toggle the
weight unit through the screen, and assert the updated label using the widget
finder pattern established by the first test in the file, while retaining the
existing persistence and provider assertions.
🪄 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 Plus
Run ID: 3d79a405-f0a6-4cca-b623-5f24324ee083
📒 Files selected for processing (11)
workout-logger/test/api_service_test.dartworkout-logger/test/debug_log_buffer_test.dartworkout-logger/test/gemini_context_builder_test.dartworkout-logger/test/settings_provider_test.dartworkout-logger/test/sleep_hr_builder_test.dartworkout-logger/test/sleep_hr_models_test.dartworkout-logger/test/storage_service_test.dartworkout-logger/test/userflow_history_and_session_details_test.dartworkout-logger/test/userflow_routine_creation_test.dartworkout-logger/test/userflow_settings_and_storage_test.dartworkout-logger/test/userflow_workout_logging_test.dart
Comment on lines
+64
to
+76
| testWidgets('Toggling weight unit in SettingsProvider persists to storage and updates display label', (tester) async { | ||
| expect(settingsProvider.weightUnit, equals(WeightUnit.kg)); | ||
| expect(settingsProvider.unitLabel, equals('kg')); | ||
|
|
||
| await settingsProvider.setWeightUnit(WeightUnit.lbs); | ||
| expect(settingsProvider.weightUnit, equals(WeightUnit.lbs)); | ||
| expect(settingsProvider.unitLabel, equals('lbs')); | ||
| expect(mockStorage.settings['weightUnit'], equals('lbs')); | ||
|
|
||
| await settingsProvider.setWeightIncrement(5.0); | ||
| expect(settingsProvider.weightIncrement, equals(5.0)); | ||
| expect(mockStorage.settings['weightIncrement'], equals('5.0')); | ||
| }); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Test doesn't verify the "display label" update it claims to.
This test only exercises SettingsProvider directly (no pumpWidget call), so the described UI-label-update behavior is never actually checked against rendered output. Consider rendering SettingsScreen and asserting the updated unit label appears (e.g. via find.text), similar to the first test in this file.
🤖 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 `@workout-logger/test/userflow_settings_and_storage_test.dart` around lines 64
- 76, The test “Toggling weight unit in SettingsProvider persists to storage and
updates display label” only checks provider state, not rendered UI. Update this
test to render SettingsScreen with the test harness, toggle the weight unit
through the screen, and assert the updated label using the widget finder pattern
established by the first test in the file, while retaining the existing
persistence and provider assertions.
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workout-logger/test/sleep_hr_builder_test.dart (1)
80-82: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the calculated sleep values, not only non-emptiness.
This test is named as a calculation test but would pass with incorrect values. Assert representative segment and deep-stage statistics such as min/max BPM, stage, and sample count.
🤖 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 `@workout-logger/test/sleep_hr_builder_test.dart` around lines 80 - 82, Strengthen the calculation test assertions after the existing snapshot checks by verifying representative calculated values in snapshot.segments and snapshot.stageStats, including expected minimum and maximum BPM, sleep stage, and sample count. Use the fixture’s known expected values so the test fails when calculations are incorrect rather than merely empty.
🤖 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 `@workout-logger/test/sleep_hr_builder_test.dart`:
- Line 9: Update the granted HealthReadType set in the test to include
HealthReadType.restingHeartRate, then assert that the resulting
buildHrDaySnapshot value has restingBpm equal to 58. Apply the same fixture and
assertion adjustment to the related test ranges so the resting-HR branch is
exercised consistently.
In `@workout-logger/test/test_utils/stub_health_connect_service.dart`:
- Around line 16-20: Update the read methods in the health-connect stub,
including readSleepSessions, readHeartRateSamples, and readRestingHeartRate, to
return fresh list copies rather than the backing fixture lists. Preserve the
existing fixture contents while preventing callers such as buildHrDaySnapshot
from mutating shared test state.
- Around line 9-13: Update the StubHcService constructor to be const, preserving
its existing parameters and compile-time default values.
---
Outside diff comments:
In `@workout-logger/test/sleep_hr_builder_test.dart`:
- Around line 80-82: Strengthen the calculation test assertions after the
existing snapshot checks by verifying representative calculated values in
snapshot.segments and snapshot.stageStats, including expected minimum and
maximum BPM, sleep stage, and sample count. Use the fixture’s known expected
values so the test fails when calculations are incorrect rather than merely
empty.
🪄 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 Plus
Run ID: 621e3f68-e3d6-4464-94da-9fa1b10efecc
📒 Files selected for processing (7)
workout-logger/lib/screens/workout_flow_screen.dartworkout-logger/test/sleep_hr_builder_test.dartworkout-logger/test/sleep_hr_models_test.dartworkout-logger/test/test_utils/stub_health_connect_service.dartworkout-logger/test/userflow_history_and_session_details_test.dartworkout-logger/test/userflow_settings_and_storage_test.dartworkout-logger/test/userflow_workout_logging_test.dart
| import 'test_utils/stub_health_connect_service.dart'; | ||
|
|
||
| void main() { | ||
| final granted = <HealthReadType>{HealthReadType.heartRate, HealthReadType.sleep}; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exercise the resting-HR branch instead of supplying dead fixture data.
Line 9 omits HealthReadType.restingHeartRate, so buildHrDaySnapshot never reads the resting fixture and falls back to the ordinary HR samples. Add that permission and assert snapshot.restingBpm == 58 so this test verifies the behavior named in its description.
Proposed test adjustment
- final granted = <HealthReadType>{HealthReadType.heartRate, HealthReadType.sleep};
+ final granted = <HealthReadType>{
+ HealthReadType.heartRate,
+ HealthReadType.sleep,
+ HealthReadType.restingHeartRate,
+ };
...
expect(snapshot!.minBpm, equals(70));
expect(snapshot.maxBpm, equals(120));
+ expect(snapshot.restingBpm, equals(58));
expect(snapshot.buckets, isNotEmpty);Also applies to: 29-37, 41-44
🤖 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 `@workout-logger/test/sleep_hr_builder_test.dart` at line 9, Update the granted
HealthReadType set in the test to include HealthReadType.restingHeartRate, then
assert that the resulting buildHrDaySnapshot value has restingBpm equal to 58.
Apply the same fixture and assertion adjustment to the related test ranges so
the resting-HR branch is exercised consistently.
Comment on lines
+9
to
+13
| StubHcService({ | ||
| this.sleepPeriods = const [], | ||
| this.hrSamples = const [], | ||
| this.restingHrSamples = const [], | ||
| }); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Make the test stub constructor const.
All fields are final and the default values are compile-time constants, so this constructor can be declared const, as required by the Dart guidelines.
Proposed fix
- StubHcService({
+ const StubHcService({📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| StubHcService({ | |
| this.sleepPeriods = const [], | |
| this.hrSamples = const [], | |
| this.restingHrSamples = const [], | |
| }); | |
| const StubHcService({ | |
| this.sleepPeriods = const [], | |
| this.hrSamples = const [], | |
| this.restingHrSamples = const [], | |
| }); |
🤖 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 `@workout-logger/test/test_utils/stub_health_connect_service.dart` around lines
9 - 13, Update the StubHcService constructor to be const, preserving its
existing parameters and compile-time default values.
Source: Coding guidelines
Comment on lines
+16
to
+20
| Future<List<SleepPeriod>> readSleepSessions(DateTime start, DateTime end) async => sleepPeriods; | ||
| @override | ||
| Future<List<HealthSample>> readHeartRateSamples(DateTime start, DateTime end) async => hrSamples; | ||
| @override | ||
| Future<List<HealthSample>> readRestingHeartRate(DateTime start, DateTime end) async => restingHrSamples; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
Return copies of fixture lists from the stub.
buildHrDaySnapshot sorts the list returned by readRestingHeartRate in place. Returning restingHrSamples directly mutates the caller’s fixture and can leak state between tests; return fresh lists for all read methods.
Proposed fix
- Future<List<SleepPeriod>> readSleepSessions(DateTime start, DateTime end) async => sleepPeriods;
+ Future<List<SleepPeriod>> readSleepSessions(DateTime start, DateTime end) async => List.of(sleepPeriods);
...
- Future<List<HealthSample>> readHeartRateSamples(DateTime start, DateTime end) async => hrSamples;
+ Future<List<HealthSample>> readHeartRateSamples(DateTime start, DateTime end) async => List.of(hrSamples);
...
- Future<List<HealthSample>> readRestingHeartRate(DateTime start, DateTime end) async => restingHrSamples;
+ Future<List<HealthSample>> readRestingHeartRate(DateTime start, DateTime end) async => List.of(restingHrSamples);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Future<List<SleepPeriod>> readSleepSessions(DateTime start, DateTime end) async => sleepPeriods; | |
| @override | |
| Future<List<HealthSample>> readHeartRateSamples(DateTime start, DateTime end) async => hrSamples; | |
| @override | |
| Future<List<HealthSample>> readRestingHeartRate(DateTime start, DateTime end) async => restingHrSamples; | |
| Future<List<SleepPeriod>> readSleepSessions(DateTime start, DateTime end) async => List.of(sleepPeriods); | |
| `@override` | |
| Future<List<HealthSample>> readHeartRateSamples(DateTime start, DateTime end) async => List.of(hrSamples); | |
| `@override` | |
| Future<List<HealthSample>> readRestingHeartRate(DateTime start, DateTime end) async => List.of(restingHrSamples); |
🤖 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 `@workout-logger/test/test_utils/stub_health_connect_service.dart` around lines
16 - 20, Update the read methods in the health-connect stub, including
readSleepSessions, readHeartRateSamples, and readRestingHeartRate, to return
fresh list copies rather than the backing fixture lists. Preserve the existing
fixture contents while preventing callers such as buildHrDaySnapshot from
mutating shared test state.
Merged
Devasy
added a commit
that referenced
this pull request
Sep 1, 2026
* Feat/android 17 (#59)
* feat: add localized summaries, app metadata, and signing block information to F-Droid repository data
* chore: upgrade Android SDK 16→17, Java 11→17, Gradle/AGP/Kotlin toolchain
- compileSdk + targetSdk: 36 → 37 (Android 17 / API 37)
- Removed compileSdkExtension (not needed for base API 37)
- Java source/target compatibility: VERSION_11 → VERSION_17
- Kotlin jvmTarget: 11 → 17
- Gradle wrapper: 8.12 → 8.14.1
- AGP: 8.9.1 → 8.11.1
- Kotlin Gradle Plugin: 2.1.0 → 2.2.20
- Enable android.builtInKotlin=true + android.newDsl=true
- Remove explicit id(kotlin-android) plugin (now injected by Flutter)
* chore: update pubspec.lock (transitive dependency bumps)
* chore: update repo name and username references to RepForge and Devasy
* upadtes the build gradle kts file to match the review comment
* Adds pubspec yaml
---------
Co-authored-by: Devasy Patel <110348311+Devasy23@users.noreply.github.com>
* Upgrades bottom nav bar Upgrade Bottom Navigation Bar (#61)
* feat: add localized summaries, app metadata, and signing block information to F-Droid repository data
* chore: upgrade Android SDK 16→17, Java 11→17, Gradle/AGP/Kotlin toolchain
- compileSdk + targetSdk: 36 → 37 (Android 17 / API 37)
- Removed compileSdkExtension (not needed for base API 37)
- Java source/target compatibility: VERSION_11 → VERSION_17
- Kotlin jvmTarget: 11 → 17
- Gradle wrapper: 8.12 → 8.14.1
- AGP: 8.9.1 → 8.11.1
- Kotlin Gradle Plugin: 2.1.0 → 2.2.20
- Enable android.builtInKotlin=true + android.newDsl=true
- Remove explicit id(kotlin-android) plugin (now injected by Flutter)
* chore: update pubspec.lock (transitive dependency bumps)
* chore: update repo name and username references to RepForge and Devasy
* upadtes the build gradle kts file to match the review comment
* Adds pubspec yaml
* Enhances the bottom nav bar
* fixes out bulging issue
* Updates the bottom navbar UI, and then adds build size reuction params
* Adds build script and upgrades the release workflow
* Adds tests
* updates acc to review comments
* Adds gitignore and updates codecov yaml
* updated comments according to review comments
---------
Co-authored-by: Devasy Patel <110348311+Devasy23@users.noreply.github.com>
* Feat/increase coverage test screens (#63)
* Adds tests for screens
* Adds tests
* Adds comprehensive tests
* Adds new tests
* Updates test.yml to run on release branches
* Adds test and resolved the warnings and issues
* Updates tests and minor bug fixes
* Adds fixes for failing testsm and adds connection timeout safety for health connector
* Adds missing lines patch
* Updates the tests with analyse failures
* Updates tests and routine creator to use the common component
* Updates flutter version and adds tests
---------
Co-authored-by: Devasy Patel <110348311+Devasy23@users.noreply.github.com>
* Feat/genui (#64)
* Adds tests for screens
* Adds tests
* Adds comprehensive tests
* Adds new tests
* Updates test.yml to run on release branches
* Adds test and resolved the warnings and issues
* Updates tests and minor bug fixes
* Adds fixes for failing testsm and adds connection timeout safety for health connector
* Adds missing lines patch
* Updates the tests with analyse failures
* Updates tests and routine creator to use the common component
* Updates flutter version and adds tests
* Adds major genui Feature and renderer
* chore: remove patch_so script
* build: add --build-id=none for jni package in F-Droid metadata
* ci: add jni build-id sed step for future reproducible releases
* feat: assisted pullups, deload-aware ML, handle-scoped PRs, sleeping HR tool
Batches several in-flight features that were sitting uncommitted:
- Bodyweight/assisted pullup volume: (BW - assist + extra) * reps
- MLService reads the past 3 sessions and recovers from a deload week
using the pre-deload baseline instead of the deload trough
- PRManager scopes records per handle variation (Rope vs Bar)
- CoachToolService.get_sleeping_hr_analytics: p5/p25/mean, stdev,
variance and linear trend over the last N nights
- GenUI parser tolerates numeric StatCard values, loose trend words and
Markdown code fences
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add A2UiProps alias-aware coercing property reader
Foundation for the genui refactor: a never-throwing view over raw
component prop maps that resolves keys by exact match, normalized
match (case/underscore/hyphen/space-insensitive), then semantic
alias, and coerces values to typed accessors with documented
fallbacks instead of throwing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add A2UiSpec contract, A2UiNode and A2UiRegistry
Adds the four-in-one component contract (A2UiSpec) that lets each UI
component name itself, parse its own props, build its own widget and
document itself for the LLM prompt on one object, plus the
A2UiRegistry lookup table that replaces the old allowedA2UiComponents
set and two parallel switch statements. Includes an A2UiTheme skeleton
(filled in by Task 4) and A2UiNode, the parsed-tree node type.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): make A2UiRegistry throw on name/alias collisions
Code review found that A2UiRegistry's constructor loop silently
resolved canonical-name/alias collisions (last-writer-wins for names,
first-writer-wins for aliases), which would produce unreachable specs
or dropped aliases with no signal as more components are registered in
later tasks. The constructor now throws a StateError identifying both
colliding specs for any of: two specs sharing a canonical name, an
alias colliding with another spec's canonical name, or two specs
sharing an alias. Adds three regression tests using a new configurable
_NamedFakeSpec fake.
Also documents (doc-comment only, no behavior change) that
A2UiNode.children is not defensively copied, per the review's Minor
finding.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add A2UiParser with fence, envelope and alias repair
Adds the single gate that decides whether an LLM reply is a UI payload
or ordinary prose, and turns UI payloads into an A2UiNode tree. Handles
markdown fences, prose-wrapped JSON, flat vs props-wrapped shapes,
bare-array/envelope auto-wrapping into GridContainer, and recursive
children, without ever throwing.
Also promotes A2UiProps._asStringKeyed to a public static
A2UiProps.stringKeyed so the parser can re-key decoded JSON maps
without an awkward part-of coupling between the two libraries.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): balanced-bracket JSON extraction and envelope singleton fix
_extractJson previously sliced from the first { to the last }, which
broke on any stray brace in surrounding prose (e.g. "add reps
{optional}"). Replace with a scan that tries jsonDecode on every
balanced {..}/[..] span found via a depth counter that correctly skips
brackets inside string literals, preferring the longest successful
decode as the actual payload.
Also fix _wrap's unconditional single-child collapse: an explicit
envelope key ({"components":[...]}) is a deliberate container request
and must still produce a GridContainer with one child, while a bare
top-level array with one item keeps collapsing since it's ambiguous
between "a list of one" and "just one component."
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): inject A2UiTheme and extract shared panel chrome
Adds A2UiThemeProvider (InheritedWidget, falls back to A2UiTheme.dark)
and the panel/title/empty-state/legend widgets every component spec
will share, plus lib/theme/a2ui_app_theme.dart mapping RepForge's real
design tokens onto A2UiTheme. This is the only file where the two
systems meet - lib/genui/ still imports nothing app-specific.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(genui): strengthen theme-injection and add A2UiPanel coverage
The injection test compared against repforgeA2UiTheme, which is
field-for-field identical to the A2UiThemeProvider.of fallback
(A2UiTheme.dark), so it passed even if the InheritedWidget lookup were
broken. Inject a fixture with distinct values instead, and assert a
sibling context still falls back to the default. Also add direct
coverage for A2UiPanel's padding, decoration, and child rendering,
previously only exercised indirectly via A2UiEmptyPanel.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add A2UiSeries as the shared categorical data shape
A2UiSeries.extract() and maxValue() give line/bar/pie and radar chart
components one common {name, values} shape to consume, so a model that
learns {labels, series} once can drive all four components.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): cover fallback path in A2UiSeries.extract, fix negative-max bug
Address code review findings on A2UiSeries:
- Add tests pinning down the series->values fallback when every series
entry drops to empty/unparseable values, and when series is an empty
list — the risky path the brief called out but left untested.
- Rename the misleading 'reads the axes alias' test; it only exercised
stringified-number coercion inside series values, not alias resolution.
- Fix maxValue() to track whether any value has been seen instead of
seeding with 0.0, so all-negative series report their true max
instead of silently clamping to 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add StatCardSpec with typed props and trend synonyms
Establishes the pattern for Tasks 7-13: a typed props record, an
A2UiSpec bundling name/aliases/doc/parseProps/buildWidget, and
never-throwing parsing that degrades to documented fallbacks.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add MetricGaugeSpec with safe progress and null value
Fixes the validator/renderer contradiction where a String value was
accepted but cast to num, and the min == max NaN sweep angle bug.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add DynamicChartSpec for line, bar and pie
Adds the most-used and most complex A2UI component so far, covering
line/bar/pie rendering over the shared {labels, series} shape with
never-throwing prop parsing and label padding to prevent out-of-range
axis lookups.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add ScatterPlotSpec with point repair and safe bounds
Adds paired x/y observation plotting with an optional correlation badge,
following the Task 6-8 A2UiSpec pattern. Malformed points are dropped
rather than throwing, and bounds widen degenerate axes so fl_chart never
sees a zero-span range.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add RadarChartSpec sharing the labels/series shape
Task 10 of the a2ui/genui refactor: RadarChart consumes the same
{labels, series} shape as DynamicChart, with `axes` kept as a
backward-compatible alias for `labels`. Every series is truncated
or zero-padded to labels.length at parse time so fl_chart's radar
never sees a mismatched entry count.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add DataListGroupSpec with row repair and optional title
Adds a titled list-of-rows component with a defensive row-extraction
fallback chain: named fields, bare scalars, first-stringifiable-value
fallback, and silent drop of rows with nothing displayable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add FilterChipsSpec with nullable active option
Renders a decorative, non-interactive row of scope chips (e.g. "7d /
30d / 90d") and fixes the old renderer's `activeOption as String`
crash by matching case-insensitively and falling back to null instead
of throwing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add GridContainerSpec, default registry and renderer
Task 13: assembles all eight leaf components into defaultA2UiRegistry,
adds the GridContainerSpec layout wrapper, the public A2UiRenderer
widget, and the lib/genui/a2ui.dart barrel file that will be the only
import path the rest of the app uses going forward.
fix(genui): make structural children lookup exact, not alias-resolved
Cross-task fix to a2ui_parser.dart (a Task 3 file), discovered during
Task 13 registry integration. A2UiParser._parseChildren and
_declaresChildren resolved the structural `children` key through
A2UiProps' alias-aware lookup(), which treats `items` as an alias for
`children`. That collided with DataListGroupSpec, whose own canonical
data-row key is also `items`: a DataListGroup node's `items` list of
{primaryText, ...} maps was mistaken for child components, none of
them parsed as one, and the whole node was then discarded as an
emptied-out container. Reading the literal `children` key only fixes
this and matches the precision _envelopeKeys already had (it does not
include `items` as a synonym for `children` either).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): generate the A2UI prompt section from the registry
Replaces hand-written component-schema prose in the coach system
prompt with a section generated from defaultA2UiRegistry, so the
vocabulary advertised to the model can never drift from what the
parser/renderer actually support.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(genui): wire coach screen to the A2UI package, drop legacy renderer
Replaces private _CoachMessageContent with a public, stateful
CoachMessageContent that memoizes parsing per text value and shows a
"Building dashboard..." placeholder for partial JSON while streaming,
instead of letting raw braces scroll past or losing prose on a mixed
reply. Wraps the app root in A2UiThemeProvider(theme: repforgeA2UiTheme)
so the renderer picks up RepForge's design tokens. Deletes the
superseded lib/genui/a2ui_component.dart and lib/genui/a2ui_renderer.dart,
and drops test/new_features_test.dart's GenUI Component Resilience Tests
group, whose two cases are already covered more thoroughly by
test/genui/a2ui_parser_test.dart.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): bracket negative-value ranges in DynamicChart line/bar axes
minY was hardcoded to 0 while maxY derived from the true series max, so an
all-negative dataset (e.g. [-10, -5, -3]) produced a visible axis range of
[0, 1] with every real data point falling outside it — a silent blank
chart despite valid, non-empty data. Adds A2UiSeries.minValue mirroring
the existing maxValue, and a shared _yBounds helper used by both _line and
_bar so the two renderers can't diverge on axis math. Also covers
multi-series label padding, which was previously only exercised through
series[0].
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(genui): cover all-negative bounds and malformed point entries
Task 9 review flagged that ScatterPlotProps.bounds had no regression pin
for all-negative-coordinate spreads (same failure class as Task 8's
DynamicChartSpec axis bug) and that point-parsing had no test for
structurally invalid entries (nested objects, raw lists). Adds both.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): widen per-node children lookup back to components/elements/content
Follow-up to the Task 13 a2ui_parser.dart fix: restricting the per-node
_parseChildren/_declaresChildren lookup to the literal 'children' key
was narrower than intended. It regressed 'components'/'elements'/
'content' as per-node child-list keys, which never collided with
anything (only 'items' did, via DataListGroup's own canonical data key).
A payload like {"component":"GridContainer","props":{"columns":1,
"components":[...]}} resolved fine before the original bug and silently
rendered blank (zero children, no null fallback) after the first fix,
since _declaresChildren no longer recognized 'components' as a
children-declaring key either.
Adds a _childKeys constant (children/components/elements/content,
still excluding items) mirroring _envelopeKeys' existing tolerance, and
routes both _parseChildren and _declaresChildren through a shared
_firstChildList literal (non-alias) lookup over that key set.
Adds regression tests in a2ui_renderer_test.dart: per-node
components/elements/content resolve to real children, and items stays
excluded at the per-node level.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): widen looksLikeUi to catch prose-prefixed fences, fix vacuous memoization test
looksLikeUi only checked whether the text, after stripping a *leading*
fence, started with `{`/`[`. A model that writes a sentence before
opening a fenced payload (e.g. "Here is your data:\n```json\n{...")
fell through undetected, so CoachMessageContent showed the raw partial
JSON instead of the streaming placeholder -- the exact symptom this
task exists to fix. Now also treats an unclosed ``` fence found
anywhere in the streamed-so-far text as a UI signal, while plain prose
with no JSON or fence anywhere still returns false.
Also fixes the memoization regression test in
test/screens/ai_coach_genui_test.dart: the second observation was
taken after a bare `tester.pump()`, which doesn't mark the element
dirty and never actually calls build() again, so the test could not
distinguish memoized parsing from a widget that never rebuilds at all.
It now pumps a second CoachMessageContent instance with identical text
at the same tree location, which reuses the existing State and
genuinely triggers didUpdateWidget/build.
Adds regression tests for both the prose-prefixed-fence case and the
plain-prose-no-json case in test/genui/a2ui_parser_test.dart, plus a
widget-level test in test/screens/ai_coach_genui_test.dart confirming
the placeholder (not raw JSON) renders end-to-end.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(genui): drop presentation payload from tools, add purity and fuzz suites
The sleeping-HR analytics tool was hand-constructing an A2UI DynamicChart
payload directly, leaking presentation decisions into the data layer.
Replace `genui_chart_props` with neutral `labels`/`series` keys so the
prompt — not the tool — decides how to present the data.
Add two permanent guard suites: a2ui_purity_test.dart proves lib/genui/
never imports app-specific code (theme/models/services/screens) and its
component renderers never cast raw model data; a2ui_robustness_test.dart
fuzzes the parser and renderer against ~26 hostile/malformed LLM payloads
to confirm nothing throws.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): depth-agnostic purity regex, pin two silent-visual regressions
Review of the previous commit found the purity test's forbidden-import
check was depth-blind: its literal needle list only covered one and two
../ hops, but components live three levels below lib/, so a real
../../../theme/... import passed undetected. Replace it with a regex
that matches any number of ../ hops (or a package:repforge/ prefix),
covering import and export directives alike, and add a self-test that
proves the regex catches every relevant depth/form without touching real
source files.
Also widen the no-raw-casts check to include bool/Object/dynamic, make
the components-directory scan recursive, and pin down the two historical
silent-visual regressions (Task 8's chart axis-bounds clamp, Task 13's
GridContainer child-key aliasing) with positive assertions in the fuzz
suite, since neither throws and the existing no-throw checks structurally
can't catch either.
Reword analyze_health_workout_correlation's tool declaration to drop
direct component names, closing the same presentation-leak class this
task already fixed for the sleeping-HR tool.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): propagate registry through recursion, pin prompt drift, close review findings
Final whole-branch review fix wave for the A2UI genui refactor:
- A2UiRenderer's registry override used to be silently dropped past one
level of nesting because GridContainerSpec recurses via bare
A2UiRenderer(node: ...) calls. Mirror the existing theme-injection
pattern with a new A2UiRegistryProvider InheritedWidget so an explicit
registry override at any level propagates ambiently to everything below
it (explicit param > inherited provider > defaultA2UiRegistry fallback).
- Pin the hand-written "WHICH COMPONENT TO REACH FOR" prose in
gemini_context_builder.dart against silent drift: every component name
it mentions must resolve in defaultA2UiRegistry, and the registry's
spec count is asserted directly.
- Delete A2UiProps.object()/has() — confirmed zero call sites.
- Repurpose the orphaned Task 3 scaffolding test
(a2ui_parser_stub_test.dart, redundant with a2ui_parser_test.dart) into
a2ui_custom_registry_test.dart, the regression coverage the registry-
propagation fix needed.
- Add scanned-file-count floors to the purity test's two directory scans
so an empty/unreachable directory can't produce a vacuous pass.
- Document FilterChips' SizedBox.shrink() as a deliberate exception to
the plan's "always A2UiEmptyPanel" rule (decorative chrome, not data).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs: add design spec for Hive->SQLite migration + coach SQL query tool
* fix: persist assisted-load volume correctly, tighten exercise-handle scoping
- WorkoutSet now snapshots bodyweight/assist/extra at logging time instead
of recomputing effective load from the CURRENT profile bodyweight on every
read, which was silently corrupting historical volume whenever a user
updated their weight. ExerciseLog.totalVolume and the workout_flow_screen
logging path thread the snapshot through.
- Exercise-handle matching (workout_provider) now requires an exact handle
match whenever a handle is set, falling back to legacy behavior only when
no exact match exists — a null-handle log was previously matching ANY
requested handle, surfacing the wrong variation's "last session" data.
- Handle selector no longer visually pre-selects an unpersisted handle, and
setExerciseHandle no longer retroactively relabels already-logged sets.
- Assisted-load display values now respect the user's unit preference; the
assisted-exercise classification is computed once and shared instead of
drifting between two separate predicates.
- Body-weight input (settings_provider) now rejects non-finite/non-positive
values on both the load and set paths, falling back to 70.0 when invalid.
- ml_service: deload-recovery reasoning no longer hardcodes "kg" regardless
of unit settings; recovery detection now requires the comparison session
to be recent and uses effective (not raw) load for assisted exercises.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix: bound sleep-analytics window, drop fabricated data, resolve muscle groups by id
- get_sleeping_hr_analytics clamps the model-provided days window instead of
looping unbounded; get_health_metrics now honors the requested days window
instead of always querying one week, and both its and the correlation
tool's declarations no longer advertise fields (resting HR, readiness)
that aren't actually backed by implementation.
- analyze_health_workout_correlation no longer fabricates synthetic sleep
data points to pad out insufficient real pairs — returns the existing
insufficient-data error instead, so correlation/regression/chart output is
never partly made up.
- get_muscle_group_volume now resolves requested names to ids via
_resolveMuscleGroup and compares ids (also aggregating secondary muscle
activations) instead of raw display-name substring matching.
- CoachToolService's optional HealthHistoryManager is now a named parameter.
- gemini_ai_service: daily-quota classification narrowed to actual
daily-limit identifiers so minute-scale rate limits go through normal
retry-delay handling instead of being misclassified as daily exhaustion;
function-call ids are now preserved and matched into their responses;
the fallback path now builds a thinkingConfig compatible with whichever
model was actually selected. Mirrored in scripts/test_gemini_api.py.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): pie negative-value filtering, overflow guard, stat-card unit match
- DynamicChart's pie mode now filters to positive values before computing
percentages/sections (preserving original index alignment with labels and
series colors), falling back to an empty panel when nothing positive
remains, instead of rendering a nonsense chart from negative/zero data.
- A2UiPanelTitle's trailing label is now Flexible with maxLines/ellipsis so
a long model-provided string can't overflow the row.
- StatCard's unit-already-present check now requires a trailing-suffix
match instead of any substring, fixing a false positive like unit "s"
matching inside value "10 reps".
- MetricGauge's arc painter now also compares `track` in shouldRepaint, so
a background-color-only change still triggers a repaint.
- A2UiTheme.seriesColor asserts a non-empty palette before the modulo index
that would otherwise throw on one.
- A2UiParser: props/outer-children now merge (props wins on conflict) so a
model writing children as a sibling of props isn't silently dropped; adds
a whole-text jsonDecode fast path ahead of the balanced-span scan.
- A2UiRenderer logs the unresolved component name via the app's existing
debugPrint/kDebugMode convention before falling back to an empty widget.
- a2ui_app_theme now imports A2UiTheme via the public genui barrel instead
of an internal src path.
- CI: the release workflow's linker-patch step now requires and quotes
PUB_CACHE, restricts the patch to resolved jni-*/src/CMakeLists.txt
targets, is idempotent against re-runs, and fails the build instead of
silently continuing when no target is found or patching fails.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test: close vacuous-test gaps and pin already-fixed regressions
Fixes tests that would pass identically whether the behavior they claim to
verify was correct or broken:
- stat_card_test's pump() helper now actually threads its props argument
into the rendered node (it previously always rendered empty props).
- new_features_test's assisted-pullups case now uses distinguishable
weight/assistWeight values, so the test fails if the wrong field is used.
Tightens two guardrail-class tests to actually detect what they claim to:
- a2ui_prompt_test's worked-example extraction is now bounded to the region
after the "WORKED EXAMPLE:" marker via balanced-brace matching, instead of
the last '}' anywhere in the whole prompt.
- a2ui_purity_test's forbidden-import regex now also guards lib/data/.
- a2ui_robustness_test's negative-axis assertion now requires minY to
actually bracket the dataset's true minimum, not just be below -10.
- a2ui_theme_test's panel-decoration finders are scoped to the panel under
test rather than the first Container anywhere in the tree.
Adds regression coverage pinning fixes already shipped in prior commits:
DynamicChart pie's negative-value filtering, StatCard's unit-suffix match,
and CoachToolService's days-window/insufficient-data/muscle-id fixes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix: address CodeRabbit review findings on PR #64 (feat/genui)
Fixes real findings from PR #64's own review, ahead of merging into
r2.1.0, so the sqflite-migration branch (which currently carries these
genui files unmerged) won't reintroduce them as merge conflicts.
- a2ui_theme: seriesColor() now falls back to accent on an empty
seriesPalette instead of only asserting (release builds strip
asserts, so this was still a release-mode divide-by-zero)
- coach_tool_service: removed the synthetic "readiness_score" metric
from analyze_health_workout_correlation — it was a made-up
70-100 formula derived from sleep duration, presented as if it were
an independent measured health signal in statistical output
- coach_tool_service, main.dart: CoachToolService constructor now uses
named parameters (3+ args); updated every call site
- workout_provider: getRecommendations no longer passes the
exercise-wide growth model into a handle-scoped recommendation,
since _growthModels isn't trained per-handle and would mix
variations (e.g. "Rope pushdown" trend bleeding into "Bar pushdown")
- test_gemini_api.py: post_generate_content_with_retry could fall off
the end returning None after a quota-fallback on the final attempt,
despite its dict return type; restructured so every path returns or
raises
- test coverage: legend-absence assertions for single-series/pie
charts, NaN/Infinity scatter-point coordinates, stable payload-based
test names in the robustness suite, hoisted regex in the purity
test, const constructor, and a corrected self-contradictory comment
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Devasy Patel <110348311+Devasy23@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* test: drop coverage for ApiService/SettingsScreen removed by main merge
Merging main brought in the telemetry removal (ApiService and the
orphaned settings_screen.dart are gone). r2.1.0 had its own test
coverage for both that main never had - api_service_test.dart,
screens/settings_screen_test.dart, and the SettingsScreen-only half
of userflow_settings_and_storage_test.dart all targeted code that no
longer exists, so they're deleted. test_harness.dart drops its
ApiService provider registration, which nothing consumes anymore.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Migrate storage from Hive to SQLite + add coach SQL query tool (#66)
* Adds tests for screens
* Adds tests
* Adds comprehensive tests
* Adds new tests
* Updates test.yml to run on release branches
* Adds test and resolved the warnings and issues
* Updates tests and minor bug fixes
* Adds fixes for failing testsm and adds connection timeout safety for health connector
* Adds missing lines patch
* Updates the tests with analyse failures
* Updates tests and routine creator to use the common component
* Updates flutter version and adds tests
* Adds major genui Feature and renderer
* chore: remove patch_so script
* build: add --build-id=none for jni package in F-Droid metadata
* ci: add jni build-id sed step for future reproducible releases
* feat: assisted pullups, deload-aware ML, handle-scoped PRs, sleeping HR tool
Batches several in-flight features that were sitting uncommitted:
- Bodyweight/assisted pullup volume: (BW - assist + extra) * reps
- MLService reads the past 3 sessions and recovers from a deload week
using the pre-deload baseline instead of the deload trough
- PRManager scopes records per handle variation (Rope vs Bar)
- CoachToolService.get_sleeping_hr_analytics: p5/p25/mean, stdev,
variance and linear trend over the last N nights
- GenUI parser tolerates numeric StatCard values, loose trend words and
Markdown code fences
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add A2UiProps alias-aware coercing property reader
Foundation for the genui refactor: a never-throwing view over raw
component prop maps that resolves keys by exact match, normalized
match (case/underscore/hyphen/space-insensitive), then semantic
alias, and coerces values to typed accessors with documented
fallbacks instead of throwing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add A2UiSpec contract, A2UiNode and A2UiRegistry
Adds the four-in-one component contract (A2UiSpec) that lets each UI
component name itself, parse its own props, build its own widget and
document itself for the LLM prompt on one object, plus the
A2UiRegistry lookup table that replaces the old allowedA2UiComponents
set and two parallel switch statements. Includes an A2UiTheme skeleton
(filled in by Task 4) and A2UiNode, the parsed-tree node type.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): make A2UiRegistry throw on name/alias collisions
Code review found that A2UiRegistry's constructor loop silently
resolved canonical-name/alias collisions (last-writer-wins for names,
first-writer-wins for aliases), which would produce unreachable specs
or dropped aliases with no signal as more components are registered in
later tasks. The constructor now throws a StateError identifying both
colliding specs for any of: two specs sharing a canonical name, an
alias colliding with another spec's canonical name, or two specs
sharing an alias. Adds three regression tests using a new configurable
_NamedFakeSpec fake.
Also documents (doc-comment only, no behavior change) that
A2UiNode.children is not defensively copied, per the review's Minor
finding.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add A2UiParser with fence, envelope and alias repair
Adds the single gate that decides whether an LLM reply is a UI payload
or ordinary prose, and turns UI payloads into an A2UiNode tree. Handles
markdown fences, prose-wrapped JSON, flat vs props-wrapped shapes,
bare-array/envelope auto-wrapping into GridContainer, and recursive
children, without ever throwing.
Also promotes A2UiProps._asStringKeyed to a public static
A2UiProps.stringKeyed so the parser can re-key decoded JSON maps
without an awkward part-of coupling between the two libraries.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): balanced-bracket JSON extraction and envelope singleton fix
_extractJson previously sliced from the first { to the last }, which
broke on any stray brace in surrounding prose (e.g. "add reps
{optional}"). Replace with a scan that tries jsonDecode on every
balanced {..}/[..] span found via a depth counter that correctly skips
brackets inside string literals, preferring the longest successful
decode as the actual payload.
Also fix _wrap's unconditional single-child collapse: an explicit
envelope key ({"components":[...]}) is a deliberate container request
and must still produce a GridContainer with one child, while a bare
top-level array with one item keeps collapsing since it's ambiguous
between "a list of one" and "just one component."
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): inject A2UiTheme and extract shared panel chrome
Adds A2UiThemeProvider (InheritedWidget, falls back to A2UiTheme.dark)
and the panel/title/empty-state/legend widgets every component spec
will share, plus lib/theme/a2ui_app_theme.dart mapping RepForge's real
design tokens onto A2UiTheme. This is the only file where the two
systems meet - lib/genui/ still imports nothing app-specific.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(genui): strengthen theme-injection and add A2UiPanel coverage
The injection test compared against repforgeA2UiTheme, which is
field-for-field identical to the A2UiThemeProvider.of fallback
(A2UiTheme.dark), so it passed even if the InheritedWidget lookup were
broken. Inject a fixture with distinct values instead, and assert a
sibling context still falls back to the default. Also add direct
coverage for A2UiPanel's padding, decoration, and child rendering,
previously only exercised indirectly via A2UiEmptyPanel.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add A2UiSeries as the shared categorical data shape
A2UiSeries.extract() and maxValue() give line/bar/pie and radar chart
components one common {name, values} shape to consume, so a model that
learns {labels, series} once can drive all four components.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): cover fallback path in A2UiSeries.extract, fix negative-max bug
Address code review findings on A2UiSeries:
- Add tests pinning down the series->values fallback when every series
entry drops to empty/unparseable values, and when series is an empty
list — the risky path the brief called out but left untested.
- Rename the misleading 'reads the axes alias' test; it only exercised
stringified-number coercion inside series values, not alias resolution.
- Fix maxValue() to track whether any value has been seen instead of
seeding with 0.0, so all-negative series report their true max
instead of silently clamping to 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add StatCardSpec with typed props and trend synonyms
Establishes the pattern for Tasks 7-13: a typed props record, an
A2UiSpec bundling name/aliases/doc/parseProps/buildWidget, and
never-throwing parsing that degrades to documented fallbacks.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add MetricGaugeSpec with safe progress and null value
Fixes the validator/renderer contradiction where a String value was
accepted but cast to num, and the min == max NaN sweep angle bug.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add DynamicChartSpec for line, bar and pie
Adds the most-used and most complex A2UI component so far, covering
line/bar/pie rendering over the shared {labels, series} shape with
never-throwing prop parsing and label padding to prevent out-of-range
axis lookups.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add ScatterPlotSpec with point repair and safe bounds
Adds paired x/y observation plotting with an optional correlation badge,
following the Task 6-8 A2UiSpec pattern. Malformed points are dropped
rather than throwing, and bounds widen degenerate axes so fl_chart never
sees a zero-span range.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add RadarChartSpec sharing the labels/series shape
Task 10 of the a2ui/genui refactor: RadarChart consumes the same
{labels, series} shape as DynamicChart, with `axes` kept as a
backward-compatible alias for `labels`. Every series is truncated
or zero-padded to labels.length at parse time so fl_chart's radar
never sees a mismatched entry count.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add DataListGroupSpec with row repair and optional title
Adds a titled list-of-rows component with a defensive row-extraction
fallback chain: named fields, bare scalars, first-stringifiable-value
fallback, and silent drop of rows with nothing displayable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add FilterChipsSpec with nullable active option
Renders a decorative, non-interactive row of scope chips (e.g. "7d /
30d / 90d") and fixes the old renderer's `activeOption as String`
crash by matching case-insensitively and falling back to null instead
of throwing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): add GridContainerSpec, default registry and renderer
Task 13: assembles all eight leaf components into defaultA2UiRegistry,
adds the GridContainerSpec layout wrapper, the public A2UiRenderer
widget, and the lib/genui/a2ui.dart barrel file that will be the only
import path the rest of the app uses going forward.
fix(genui): make structural children lookup exact, not alias-resolved
Cross-task fix to a2ui_parser.dart (a Task 3 file), discovered during
Task 13 registry integration. A2UiParser._parseChildren and
_declaresChildren resolved the structural `children` key through
A2UiProps' alias-aware lookup(), which treats `items` as an alias for
`children`. That collided with DataListGroupSpec, whose own canonical
data-row key is also `items`: a DataListGroup node's `items` list of
{primaryText, ...} maps was mistaken for child components, none of
them parsed as one, and the whole node was then discarded as an
emptied-out container. Reading the literal `children` key only fixes
this and matches the precision _envelopeKeys already had (it does not
include `items` as a synonym for `children` either).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(genui): generate the A2UI prompt section from the registry
Replaces hand-written component-schema prose in the coach system
prompt with a section generated from defaultA2UiRegistry, so the
vocabulary advertised to the model can never drift from what the
parser/renderer actually support.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(genui): wire coach screen to the A2UI package, drop legacy renderer
Replaces private _CoachMessageContent with a public, stateful
CoachMessageContent that memoizes parsing per text value and shows a
"Building dashboard..." placeholder for partial JSON while streaming,
instead of letting raw braces scroll past or losing prose on a mixed
reply. Wraps the app root in A2UiThemeProvider(theme: repforgeA2UiTheme)
so the renderer picks up RepForge's design tokens. Deletes the
superseded lib/genui/a2ui_component.dart and lib/genui/a2ui_renderer.dart,
and drops test/new_features_test.dart's GenUI Component Resilience Tests
group, whose two cases are already covered more thoroughly by
test/genui/a2ui_parser_test.dart.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): bracket negative-value ranges in DynamicChart line/bar axes
minY was hardcoded to 0 while maxY derived from the true series max, so an
all-negative dataset (e.g. [-10, -5, -3]) produced a visible axis range of
[0, 1] with every real data point falling outside it — a silent blank
chart despite valid, non-empty data. Adds A2UiSeries.minValue mirroring
the existing maxValue, and a shared _yBounds helper used by both _line and
_bar so the two renderers can't diverge on axis math. Also covers
multi-series label padding, which was previously only exercised through
series[0].
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(genui): cover all-negative bounds and malformed point entries
Task 9 review flagged that ScatterPlotProps.bounds had no regression pin
for all-negative-coordinate spreads (same failure class as Task 8's
DynamicChartSpec axis bug) and that point-parsing had no test for
structurally invalid entries (nested objects, raw lists). Adds both.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): widen per-node children lookup back to components/elements/content
Follow-up to the Task 13 a2ui_parser.dart fix: restricting the per-node
_parseChildren/_declaresChildren lookup to the literal 'children' key
was narrower than intended. It regressed 'components'/'elements'/
'content' as per-node child-list keys, which never collided with
anything (only 'items' did, via DataListGroup's own canonical data key).
A payload like {"component":"GridContainer","props":{"columns":1,
"components":[...]}} resolved fine before the original bug and silently
rendered blank (zero children, no null fallback) after the first fix,
since _declaresChildren no longer recognized 'components' as a
children-declaring key either.
Adds a _childKeys constant (children/components/elements/content,
still excluding items) mirroring _envelopeKeys' existing tolerance, and
routes both _parseChildren and _declaresChildren through a shared
_firstChildList literal (non-alias) lookup over that key set.
Adds regression tests in a2ui_renderer_test.dart: per-node
components/elements/content resolve to real children, and items stays
excluded at the per-node level.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): widen looksLikeUi to catch prose-prefixed fences, fix vacuous memoization test
looksLikeUi only checked whether the text, after stripping a *leading*
fence, started with `{`/`[`. A model that writes a sentence before
opening a fenced payload (e.g. "Here is your data:\n```json\n{...")
fell through undetected, so CoachMessageContent showed the raw partial
JSON instead of the streaming placeholder -- the exact symptom this
task exists to fix. Now also treats an unclosed ``` fence found
anywhere in the streamed-so-far text as a UI signal, while plain prose
with no JSON or fence anywhere still returns false.
Also fixes the memoization regression test in
test/screens/ai_coach_genui_test.dart: the second observation was
taken after a bare `tester.pump()`, which doesn't mark the element
dirty and never actually calls build() again, so the test could not
distinguish memoized parsing from a widget that never rebuilds at all.
It now pumps a second CoachMessageContent instance with identical text
at the same tree location, which reuses the existing State and
genuinely triggers didUpdateWidget/build.
Adds regression tests for both the prose-prefixed-fence case and the
plain-prose-no-json case in test/genui/a2ui_parser_test.dart, plus a
widget-level test in test/screens/ai_coach_genui_test.dart confirming
the placeholder (not raw JSON) renders end-to-end.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(genui): drop presentation payload from tools, add purity and fuzz suites
The sleeping-HR analytics tool was hand-constructing an A2UI DynamicChart
payload directly, leaking presentation decisions into the data layer.
Replace `genui_chart_props` with neutral `labels`/`series` keys so the
prompt — not the tool — decides how to present the data.
Add two permanent guard suites: a2ui_purity_test.dart proves lib/genui/
never imports app-specific code (theme/models/services/screens) and its
component renderers never cast raw model data; a2ui_robustness_test.dart
fuzzes the parser and renderer against ~26 hostile/malformed LLM payloads
to confirm nothing throws.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): depth-agnostic purity regex, pin two silent-visual regressions
Review of the previous commit found the purity test's forbidden-import
check was depth-blind: its literal needle list only covered one and two
../ hops, but components live three levels below lib/, so a real
../../../theme/... import passed undetected. Replace it with a regex
that matches any number of ../ hops (or a package:repforge/ prefix),
covering import and export directives alike, and add a self-test that
proves the regex catches every relevant depth/form without touching real
source files.
Also widen the no-raw-casts check to include bool/Object/dynamic, make
the components-directory scan recursive, and pin down the two historical
silent-visual regressions (Task 8's chart axis-bounds clamp, Task 13's
GridContainer child-key aliasing) with positive assertions in the fuzz
suite, since neither throws and the existing no-throw checks structurally
can't catch either.
Reword analyze_health_workout_correlation's tool declaration to drop
direct component names, closing the same presentation-leak class this
task already fixed for the sleeping-HR tool.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): propagate registry through recursion, pin prompt drift, close review findings
Final whole-branch review fix wave for the A2UI genui refactor:
- A2UiRenderer's registry override used to be silently dropped past one
level of nesting because GridContainerSpec recurses via bare
A2UiRenderer(node: ...) calls. Mirror the existing theme-injection
pattern with a new A2UiRegistryProvider InheritedWidget so an explicit
registry override at any level propagates ambiently to everything below
it (explicit param > inherited provider > defaultA2UiRegistry fallback).
- Pin the hand-written "WHICH COMPONENT TO REACH FOR" prose in
gemini_context_builder.dart against silent drift: every component name
it mentions must resolve in defaultA2UiRegistry, and the registry's
spec count is asserted directly.
- Delete A2UiProps.object()/has() — confirmed zero call sites.
- Repurpose the orphaned Task 3 scaffolding test
(a2ui_parser_stub_test.dart, redundant with a2ui_parser_test.dart) into
a2ui_custom_registry_test.dart, the regression coverage the registry-
propagation fix needed.
- Add scanned-file-count floors to the purity test's two directory scans
so an empty/unreachable directory can't produce a vacuous pass.
- Document FilterChips' SizedBox.shrink() as a deliberate exception to
the plan's "always A2UiEmptyPanel" rule (decorative chrome, not data).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs: add design spec for Hive->SQLite migration + coach SQL query tool
* fix: persist assisted-load volume correctly, tighten exercise-handle scoping
- WorkoutSet now snapshots bodyweight/assist/extra at logging time instead
of recomputing effective load from the CURRENT profile bodyweight on every
read, which was silently corrupting historical volume whenever a user
updated their weight. ExerciseLog.totalVolume and the workout_flow_screen
logging path thread the snapshot through.
- Exercise-handle matching (workout_provider) now requires an exact handle
match whenever a handle is set, falling back to legacy behavior only when
no exact match exists — a null-handle log was previously matching ANY
requested handle, surfacing the wrong variation's "last session" data.
- Handle selector no longer visually pre-selects an unpersisted handle, and
setExerciseHandle no longer retroactively relabels already-logged sets.
- Assisted-load display values now respect the user's unit preference; the
assisted-exercise classification is computed once and shared instead of
drifting between two separate predicates.
- Body-weight input (settings_provider) now rejects non-finite/non-positive
values on both the load and set paths, falling back to 70.0 when invalid.
- ml_service: deload-recovery reasoning no longer hardcodes "kg" regardless
of unit settings; recovery detection now requires the comparison session
to be recent and uses effective (not raw) load for assisted exercises.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix: bound sleep-analytics window, drop fabricated data, resolve muscle groups by id
- get_sleeping_hr_analytics clamps the model-provided days window instead of
looping unbounded; get_health_metrics now honors the requested days window
instead of always querying one week, and both its and the correlation
tool's declarations no longer advertise fields (resting HR, readiness)
that aren't actually backed by implementation.
- analyze_health_workout_correlation no longer fabricates synthetic sleep
data points to pad out insufficient real pairs — returns the existing
insufficient-data error instead, so correlation/regression/chart output is
never partly made up.
- get_muscle_group_volume now resolves requested names to ids via
_resolveMuscleGroup and compares ids (also aggregating secondary muscle
activations) instead of raw display-name substring matching.
- CoachToolService's optional HealthHistoryManager is now a named parameter.
- gemini_ai_service: daily-quota classification narrowed to actual
daily-limit identifiers so minute-scale rate limits go through normal
retry-delay handling instead of being misclassified as daily exhaustion;
function-call ids are now preserved and matched into their responses;
the fallback path now builds a thinkingConfig compatible with whichever
model was actually selected. Mirrored in scripts/test_gemini_api.py.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(genui): pie negative-value filtering, overflow guard, stat-card unit match
- DynamicChart's pie mode now filters to positive values before computing
percentages/sections (preserving original index alignment with labels and
series colors), falling back to an empty panel when nothing positive
remains, instead of rendering a nonsense chart from negative/zero data.
- A2UiPanelTitle's trailing label is now Flexible with maxLines/ellipsis so
a long model-provided string can't overflow the row.
- StatCard's unit-already-present check now requires a trailing-suffix
match instead of any substring, fixing a false positive like unit "s"
matching inside value "10 reps".
- MetricGauge's arc painter now also compares `track` in shouldRepaint, so
a background-color-only change still triggers a repaint.
- A2UiTheme.seriesColor asserts a non-empty palette before the modulo index
that would otherwise throw on one.
- A2UiParser: props/outer-children now merge (props wins on conflict) so a
model writing children as a sibling of props isn't silently dropped; adds
a whole-text jsonDecode fast path ahead of the balanced-span scan.
- A2UiRenderer logs the unresolved component name via the app's existing
debugPrint/kDebugMode convention before falling back to an empty widget.
- a2ui_app_theme now imports A2UiTheme via the public genui barrel instead
of an internal src path.
- CI: the release workflow's linker-patch step now requires and quotes
PUB_CACHE, restricts the patch to resolved jni-*/src/CMakeLists.txt
targets, is idempotent against re-runs, and fails the build instead of
silently continuing when no target is found or patching fails.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test: close vacuous-test gaps and pin already-fixed regressions
Fixes tests that would pass identically whether the behavior they claim to
verify was correct or broken:
- stat_card_test's pump() helper now actually threads its props argument
into the rendered node (it previously always rendered empty props).
- new_features_test's assisted-pullups case now uses distinguishable
weight/assistWeight values, so the test fails if the wrong field is used.
Tightens two guardrail-class tests to actually detect what they claim to:
- a2ui_prompt_test's worked-example extraction is now bounded to the region
after the "WORKED EXAMPLE:" marker via balanced-brace matching, instead of
the last '}' anywhere in the whole prompt.
- a2ui_purity_test's forbidden-import regex now also guards lib/data/.
- a2ui_robustness_test's negative-axis assertion now requires minY to
actually bracket the dataset's true minimum, not just be below -10.
- a2ui_theme_test's panel-decoration finders are scoped to the panel under
test rather than the first Container anywhere in the tree.
Adds regression coverage pinning fixes already shipped in prior commits:
DynamicChart pie's negative-value filtering, StatCard's unit-suffix match,
and CoachToolService's days-window/insufficient-data/muscle-id fixes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs: add implementation plan for Hive->SQLite migration + coach SQL tool
* chore: add sqflite dependencies for SQLite storage migration
* feat: add SqliteStorageService with schema and workout session CRUD
* fix: persist bodyWeightAtLog in SqliteStorageService sets table
* feat: implement routine and target CRUD in SqliteStorageService
* feat: implement muscle group and custom exercise CRUD in SqliteStorageService
* feat: implement settings, PR, training program, and conversation CRUD in SqliteStorageService
* feat: implement export/import in SqliteStorageService, completing IStorageService
* feat: add settings enumeration helper to StorageService for migration
* feat: add StorageMigrationService for one-time Hive-to-SQLite migration
* feat: resolve Hive-vs-SQLite storage backend in main() before runApp
* feat: add SqlQueryService for read-only SQL execution
* feat: wire run_sql_query tool into CoachToolService
* fix: fall back to fresh StorageService when app is constructed without going through main()
* fix: block run_sql_query from reading settings/sqlite_master (credential exposure)
SELECT * FROM settings or sqlite_master passed all existing run_sql_query
validation and would leak the migrated Gemini API key into model context
and persisted chat history. Add a second denylist of restricted table/
schema identifiers, checked the same way as the existing forbidden-keyword
list, plus a substring guard against SQLite's pragma_* table-valued
functions.
* fix: prevent trailing SQL comment from breaking LIMIT wrapper
A model-submitted query ending in a `--` line comment swallowed the
wrapper's closing paren when concatenated onto one line, producing an
avoidable syntax error. Put the closing `) LIMIT ?` on its own line.
Also finishes staging test/sql_query_service_test.dart, which now covers
both this fix (trailing-comment query succeeds) and the settings/
sqlite_master restricted-table rejections from the previous commit.
* docs: warn model against SELECT * across joins in run_sql_query
sqflite's row maps are keyed by column name, so a natural join query like
"SELECT * FROM sessions s JOIN exercise_logs l ON ..." silently drops
duplicate columns (e.g. id, notes) from one side with no error. Steer the
model's generated SQL toward explicit aliased columns instead.
* refactor: extract testable storage backend resolution logic; guard sqliteStorage.init()
- lib/main.dart: sqliteStorage.init() was outside the try/catch on the
path every existing user hits on first launch after this update —
disk-space/sandbox/SQLite-build failures propagated out of main()
before runApp(), so the app never booted even though the working Hive
storage right above it was fine. Now guarded with its own fallback to
Hive. Also documents why Hive.initFlutter() stays unconditional post-
cutover: ApiService reads/writes an installation id directly against
this settings box, independent of IStorageService.
- lib/services/storage_backend_resolver.dart (new): extracts the
Hive-vs-SQLite decision (migrate-or-fallback, flag write) out of
main.dart's untestable _resolveStorageBackend into a pure, directly
testable top-level function.
- test/storage_backend_resolver_test.dart (new): covers the two
real-world paths every user takes — already-migrated relaunch, and
fresh-install migration success. The forced-migration-failure case is
intentionally omitted; there's no way to make
StorageMigrationService.migrate() throw with SqliteStorageService's
current public API without adding production surface purely for
testability, and that path is exercised indirectly by
storage_migration_service_test.dart.
* docs: add design spec for syncing sleep/HR data into SQLite for coach SQL joins
Lets run_sql_query join workout data against sleep/HR history instead of
requiring separate live Health Connect tool calls per question.
* docs: add implementation plan for syncing sleep/HR data into SQLite
Five-task TDD plan: schema + upsert methods, HealthDataSyncService,
launch-time wiring, manual sync button, and the coach's schema description.
* feat: add health_samples/sleep_sessions tables + upsert methods to SqliteStorageService
- Add schema v2 with three new tables: health_samples, sleep_sessions, sleep_stage_intervals
- Add upsertHealthSamples() and upsertSleepSessions() methods for health data sync
- Add onUpgrade callback for v1->v2 schema migration
- Use temporary files for in-memory test databases to support read-only connections
- All tests passing (35/35)
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: prevent run_sql_query from closing the app's shared database connection
openReadOnlyDatabase(path) with the default singleInstance:true returns the
app's existing shared connection when called against the same path as
SqliteStorageService's live database, so the coach's per-query
finally { db.close() } was tearing down the app's only connection after
the first query. Pass singleInstance:false to force a genuinely separate
connection.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: address Task 1 review findings
- Remove _isTestDatabase path-substring flag; production init() no longer
branches on test-fixture path content
- Remove the unconditional health-schema fallback loop that made onUpgrade
untested/redundant; onCreate and onUpgrade are now the only paths that
create the health tables
- Revert IF NOT EXISTS back to plain CREATE TABLE/CREATE INDEX, matching
the existing schema statement convention
- Use a const list spread (..._healthSchemaStatements) instead of a
duplicated inline copy in _schemaStatements
- :memory: overrides still resolve to temp files (needed for read-only
secondary connections in tests), but now via an explicit Finalizer-based
cleanup keyed on the constructor's _databasePathOverride parameter
rather than sniffing the resulting path string
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: replace Finalizer with deterministic tearDown cleanup
- Remove Finalizer mechanism and unused imports (dart:async)
- Remove _tempDatabasePath and _generatedTempPath fields
- Simplify init() to convert :memory: to temp files without tracking
- Add deterministic tearDown() in test to close database and delete temp files
- Verified: no temp file leaks, all 35 tests passing
Closes: finding #5 from previous review
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* feat: add HealthDataSyncService to pull sleep/HR data into SQLite
* feat: sync health data into SQLite once per app launch
Wires HealthDataSyncService into the composition root, guarded to
only exist post-SQLite-cutover (mirrors the CoachToolService sqlQuery
guard). Fired fire-and-forget from AppInitializer._initializeApp()
alongside readiness.refresh() so it never blocks app startup.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* feat: add manual 'Sync coach data now' action to Profile screen
Lets the user force a Health Connect -> coach SQLite sync on demand
from the Health Connect section, instead of waiting for the next
app launch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* feat: teach run_sql_query about the new health_samples/sleep_sessions tables
Extends the schema description in CoachToolService's run_sql_query
declaration with health_samples, sleep_sessions, and
sleep_stage_intervals so the coach LLM knows these tables exist and
can join against them. Adds a test asserting the description text
mentions the new tables (nothing else would catch a typo/omission
there), plus a regression test for the join shape the coach will run.
* fix: remove overly broad auto-close from init, add explicit close to upgrade test
- Remove auto-close block from init() that was closing database for any
explicit file path, breaking coach_tool_service_test and other callers
- Add explicit await upgraded.close() in upgrade test before file deletion
- Regression: coach_tool_service_test now passes again
- All related tests verified: sqlite_storage_service (35), coach_tool_service (11),
health_data_sync_service (6), sql_query_service (10)
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: address final review findings for health sync + coach SQL tool
- Skip syncing a health stream entirely when its HealthReadType isn't
granted, and leave its watermark untouched — prevents watermarks
from silently advancing to `now` on first launch before the user
has opted into Health Connect, which was breaking the 90-day
backfill for essentially every user.
- Store health_samples/sleep_sessions timestamps as local time
(.toLocal() before .toIso8601String()) to match the local-naive
convention used by `sessions.date`, fixing day-bucketing joins for
non-UTC users.
- Wrap the already-migrated SQLite init() branch in main.dart with a
Hive fallback, mirroring the fresh-migration branch, so a partial
upgrade failure can't crash app startup.
- Add IF NOT EXISTS to the health-schema DDL so a retried onUpgrade
after a partial failure doesn't blow up on already-created tables.
- Add missing tearDown to health_data_sync_service_test.dart to stop
leaking temp db files, guard a profile_screen snackbar with mounted
for consistency, and reset _initialized on close() so a
close()+init() cycle actually reopens the connection.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: address migration/SQL-tool review findings from PR #66
Fixes CodeRabbit findings scoped to the hive->sqflite migration and
coach SQL tool work on this branch (genui and docs findings deferred
to their own branches):
- gemini_ai_service: rebuild generationConfig.thinkingConfig after a
daily-quota model fallback, so the retried request matches whichever
model it's about to hit instead of the previous model's shape
- health_data_sync_service: named constructor/_syncSamples params;
guard grantedReadTypes() so a Health Connect failure doesn't abort
the whole sync instead of degrading per-stream
- ml_service: recommendSets now falls back to the first non-empty
pastSessions entry when lastSession is empty, instead of returning
no recommendations
- sqlite_storage_service: guard close() against a never-initialized
db; filter getCustomExercises() by is_custom; order exercise_logs/
sets by rowid instead of the synthetic text id, which sorted "_10"
before "_2" and silently misordered sets/exercises past 9 per group
- workout_provider: removeLastSet preserves the exercise log's handle;
handle-fallback lookups only match legacy handle-less logs instead
of any handle
- test_gemini_api.py: clamp the parsed retry delay to match the Dart
implementation's bounds
- add coverage: 11+ set/exercise ordering, migration-failure fallback
path, training-program/growth-rate migration, and the id/type-only
storage-service call sites
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix: address second round of CodeRabbit findings on PR #66
Fixes real findings from the fresh review CodeRabbit ran after…
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Quality