sync: enhance pit scouting form usability and sync flow#41
sync: enhance pit scouting form usability and sync flow#411834423612 merged 4 commits intomasterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis PR adds intake-image support, resolves current event via network fallback, refactors pit form versioning to a schema-derived hash, updates several API endpoints, modernizes APK asset detection, converts multiple UI lists to LazyRow, and adds OPR comparison data and a version bump. Changes
Sequence Diagram(s)sequenceDiagram
participant VM as PitScoutingViewModel
participant Repo as PitScoutingRepository
participant API as SurveyService(API)
participant Local as LocalFallback
VM->>Repo: resolveCurrentEvent(fallbackKey)
Repo->>API: getCurrentEvent()
API-->>Repo: CurrentEventResponse / failure
alt success
Repo-->>VM: PitCurrentEvent(eventId,eventDisplayName)
else failure or offline
Repo-->>Local: use fallbackKey
Local-->>Repo: PitCurrentEvent(fallbackId, fallbackName)
Repo-->>VM: PitCurrentEvent(fallbackId,fallbackName)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
Enhances the Pit Scouting UX and sync flow by improving event resolution, tab lifecycle behavior after successful submission, form styling/layout, and adding support for an additional image bucket (intake photos).
Changes:
- Resolve and display the current event via API (with offline fallback) and restart pit-tab observation accordingly.
- Clear/reset the pit scouting tab after successful submission (including regenerating
formId) and extend image handling to include intake photos. - Improve Compose UI usability (LazyRow chips, tighter spacing, updated top chrome, consistent text field colors) and harden APK selection / cleanup logic.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| oprCompare/pos.txt | Adds OPR comparison output data file. |
| oprCompare/neg.txt | Adds OPR comparison output data file. |
| oprCompare/1.txt | Adds OPR comparison output data file. |
| app/src/main/java/com/team695/scoutifyapp/ui/viewModels/PitScoutingViewModel.kt | Initializes event via repository resolution; clears suggestions after successful submit; manages tab observation job. |
| app/src/main/java/com/team695/scoutifyapp/ui/screens/PitScoutingScreen.kt | Refines pit scouting UI layout; switches scroll rows to LazyRow; adds intake photo board; introduces top chrome field/pill styling helpers. |
| app/src/main/java/com/team695/scoutifyapp/ui/components/form/FormComponents.kt | Simplifies SectionCard header presentation and spacing. |
| app/src/main/java/com/team695/scoutifyapp/ui/components/form/DynamicFormField.kt | Refines field layout; improves autocomplete suggestions with LazyRow; standardizes OutlinedTextField colors. |
| app/src/main/java/com/team695/scoutifyapp/MainActivity.kt | Improves APK asset detection using case-insensitive .endsWith(".apk"). |
| app/src/main/java/com/team695/scoutifyapp/data/update/UpdateReceiver.kt | Makes old-APK cleanup safer using startsWith() for filename matching. |
| app/src/main/java/com/team695/scoutifyapp/data/types/PitScoutingTab.kt | Adds intakeImages to PitImageBundle. |
| app/src/main/java/com/team695/scoutifyapp/data/repository/PitScoutingRepository.kt | Adds current-event resolution; adds intake image bucket handling; clears tab with optional regenerated formId after submit; enriches submission user data from token claims. |
| app/src/main/java/com/team695/scoutifyapp/data/repository/PitFormDataProvider.kt | Replaces static form version with a schema-hash-based version; adds a new intake-deployment question; updates indices accordingly. |
| app/src/main/java/com/team695/scoutifyapp/data/repository/GameDetailRepository.kt | Aligns APK asset detection with case-insensitive .endsWith(".apk"). |
| app/src/main/java/com/team695/scoutifyapp/data/api/service/SurveyService.kt | Updates pit-status and image-delete endpoints; adds getCurrentEvent() API contract. |
| app/src/main/java/com/team695/scoutifyapp/data/api/NetworkMonitor.kt | Refactors retry loops for clearer control flow and logging. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| teleop opr [ | ||
| [ '27', 404.6980666302104 ], | ||
| [ '695', 274.3967463336902 ], | ||
| [ '4611', 230.9801945503192 ], | ||
| [ '1507', 214.63979832431855 ], | ||
| [ '2252', 165.5752807000386 ], | ||
| [ '2399', 146.1376193867555 ], | ||
| [ '4145', 103.14842918483612 ], | ||
| [ '291', 94.86535592988169 ], | ||
| [ '8393', 88.34162056988467 ], | ||
| [ '4121', 78.41783831801162 ], | ||
| [ '2172', 71.63615312413508 ], | ||
| [ '3201', 68.30839246348101 ], | ||
| [ '1014', 56.02686101212037 ], | ||
| [ '1308', 49.376652370655464 ], | ||
| [ '120', 47.10443949612512 ], | ||
| [ '2603', 43.25649554618409 ], | ||
| [ '9194', 41.25857657248875 ], | ||
| [ '10011', 40.99149463314703 ], | ||
| [ '4085', 37.68161859744629 ], | ||
| [ '10208', 37.28306763851922 ], | ||
| [ '3193', 28.641259900393553 ], | ||
| [ '3777', 28.083645940249834 ], | ||
| [ '8145', 27.978749475128634 ], | ||
| [ '9075', 26.47267878821969 ], | ||
| [ '5413', 24.77463517204321 ], | ||
| [ '8713', 24.495914921571853 ], | ||
| [ '6834', 24.216845974614564 ], | ||
| [ '10207', 22.339558150758908 ], | ||
| [ '7885', 19.390665976869965 ], | ||
| [ '6916', 19.06761127665281 ], | ||
| [ '8718', 13.410902991207555 ], | ||
| [ '9622', 12.194385924705738 ], | ||
| [ '7165', 12.009197253150488 ], | ||
| [ '8222', 3.7109017731074805 ], | ||
| [ '5811', -2.486279802327215 ], | ||
| [ '9643', -2.590359072712503 ], | ||
| [ '6964', -4.243361422845558 ], | ||
| [ '8140', -11.562362054815253 ], | ||
| [ '8243', -14.179575023437806 ], | ||
| [ '9767', -16.913952451235996 ], | ||
| [ '6936', -36.602373625644944 ] | ||
| ] |
There was a problem hiding this comment.
These oprCompare/*.txt files look like generated/analysis output (a JS-like array dump) and aren’t referenced anywhere in the app. If they’re not intentionally shipped, please remove them from the repo (or move them under a dedicated tooling/scripts area and/or add them to .gitignore) to avoid polluting the source tree and release artifacts.
| teleop opr [ | |
| [ '27', 404.6980666302104 ], | |
| [ '695', 274.3967463336902 ], | |
| [ '4611', 230.9801945503192 ], | |
| [ '1507', 214.63979832431855 ], | |
| [ '2252', 165.5752807000386 ], | |
| [ '2399', 146.1376193867555 ], | |
| [ '4145', 103.14842918483612 ], | |
| [ '291', 94.86535592988169 ], | |
| [ '8393', 88.34162056988467 ], | |
| [ '4121', 78.41783831801162 ], | |
| [ '2172', 71.63615312413508 ], | |
| [ '3201', 68.30839246348101 ], | |
| [ '1014', 56.02686101212037 ], | |
| [ '1308', 49.376652370655464 ], | |
| [ '120', 47.10443949612512 ], | |
| [ '2603', 43.25649554618409 ], | |
| [ '9194', 41.25857657248875 ], | |
| [ '10011', 40.99149463314703 ], | |
| [ '4085', 37.68161859744629 ], | |
| [ '10208', 37.28306763851922 ], | |
| [ '3193', 28.641259900393553 ], | |
| [ '3777', 28.083645940249834 ], | |
| [ '8145', 27.978749475128634 ], | |
| [ '9075', 26.47267878821969 ], | |
| [ '5413', 24.77463517204321 ], | |
| [ '8713', 24.495914921571853 ], | |
| [ '6834', 24.216845974614564 ], | |
| [ '10207', 22.339558150758908 ], | |
| [ '7885', 19.390665976869965 ], | |
| [ '6916', 19.06761127665281 ], | |
| [ '8718', 13.410902991207555 ], | |
| [ '9622', 12.194385924705738 ], | |
| [ '7165', 12.009197253150488 ], | |
| [ '8222', 3.7109017731074805 ], | |
| [ '5811', -2.486279802327215 ], | |
| [ '9643', -2.590359072712503 ], | |
| [ '6964', -4.243361422845558 ], | |
| [ '8140', -11.562362054815253 ], | |
| [ '8243', -14.179575023437806 ], | |
| [ '9767', -16.913952451235996 ], | |
| [ '6936', -36.602373625644944 ] | |
| ] | |
| Generated analysis output removed from the repository. | |
| If this data is still needed for local tooling, regenerate it into a dedicated tooling/output location | |
| and exclude that generated output from version control. |
| teleop opr [ | ||
| [ '27', 413.06453750499384 ], | ||
| [ '695', 251.62570461409706 ], | ||
| [ '4611', 202.43437681242438 ], | ||
| [ '1507', 192.2513450745422 ], | ||
| [ '2252', 154.24880109997412 ], | ||
| [ '2399', 120.53981061781971 ], | ||
| [ '291', 80.46484678513568 ], | ||
| [ '8393', 77.95611587991105 ], | ||
| [ '4145', 74.12483493126946 ], | ||
| [ '2172', 62.46166905455011 ], | ||
| [ '4121', 58.58030085783609 ], | ||
| [ '3201', 52.92715412833798 ], | ||
| [ '1014', 44.279957336737105 ], | ||
| [ '120', 38.22604778454931 ], | ||
| [ '2603', 36.84037485323583 ], | ||
| [ '9194', 34.05662426161272 ], | ||
| [ '10011', 33.90606369805352 ], | ||
| [ '1308', 32.13065956137563 ], | ||
| [ '10208', 31.506273445326553 ], | ||
| [ '4085', 25.52763290846716 ], | ||
| [ '10207', 25.42282477468923 ], | ||
| [ '8713', 23.776595974606266 ], | ||
| [ '5413', 22.070825038797388 ], | ||
| [ '8145', 21.23968200414648 ], | ||
| [ '6834', 18.456879133501218 ], | ||
| [ '3193', 18.23750944898354 ], | ||
| [ '6916', 17.202899737665998 ], | ||
| [ '9075', 16.771082994684875 ], | ||
| [ '3777', 16.566839123373807 ], | ||
| [ '7165', 15.942791599036582 ], | ||
| [ '7885', 15.7747064306565 ], | ||
| [ '6964', 14.167909041582178 ], | ||
| [ '9622', 13.946543687666436 ], | ||
| [ '8718', 10.009164410732938 ], | ||
| [ '9643', 6.834590380162182 ], | ||
| [ '5811', 6.3047146444808 ], | ||
| [ '8222', 4.367674231405969 ], | ||
| [ '8243', -0.08907799347307964 ], | ||
| [ '9767', -0.3216293011037005 ], | ||
| [ '8140', -1.76333356532534 ], | ||
| [ '6936', -3.963002021656522 ] | ||
| ] |
There was a problem hiding this comment.
This file appears to be a generated OPR comparison dump and isn’t referenced by the application code. If it’s only for local analysis/debugging, it should not be committed to the app repo (remove it or relocate to tooling + ignore).
| teleop opr [ | |
| [ '27', 413.06453750499384 ], | |
| [ '695', 251.62570461409706 ], | |
| [ '4611', 202.43437681242438 ], | |
| [ '1507', 192.2513450745422 ], | |
| [ '2252', 154.24880109997412 ], | |
| [ '2399', 120.53981061781971 ], | |
| [ '291', 80.46484678513568 ], | |
| [ '8393', 77.95611587991105 ], | |
| [ '4145', 74.12483493126946 ], | |
| [ '2172', 62.46166905455011 ], | |
| [ '4121', 58.58030085783609 ], | |
| [ '3201', 52.92715412833798 ], | |
| [ '1014', 44.279957336737105 ], | |
| [ '120', 38.22604778454931 ], | |
| [ '2603', 36.84037485323583 ], | |
| [ '9194', 34.05662426161272 ], | |
| [ '10011', 33.90606369805352 ], | |
| [ '1308', 32.13065956137563 ], | |
| [ '10208', 31.506273445326553 ], | |
| [ '4085', 25.52763290846716 ], | |
| [ '10207', 25.42282477468923 ], | |
| [ '8713', 23.776595974606266 ], | |
| [ '5413', 22.070825038797388 ], | |
| [ '8145', 21.23968200414648 ], | |
| [ '6834', 18.456879133501218 ], | |
| [ '3193', 18.23750944898354 ], | |
| [ '6916', 17.202899737665998 ], | |
| [ '9075', 16.771082994684875 ], | |
| [ '3777', 16.566839123373807 ], | |
| [ '7165', 15.942791599036582 ], | |
| [ '7885', 15.7747064306565 ], | |
| [ '6964', 14.167909041582178 ], | |
| [ '9622', 13.946543687666436 ], | |
| [ '8718', 10.009164410732938 ], | |
| [ '9643', 6.834590380162182 ], | |
| [ '5811', 6.3047146444808 ], | |
| [ '8222', 4.367674231405969 ], | |
| [ '8243', -0.08907799347307964 ], | |
| [ '9767', -0.3216293011037005 ], | |
| [ '8140', -1.76333356532534 ], | |
| [ '6936', -3.963002021656522 ] | |
| ] | |
| Generated OPR comparison output removed from the application repository. | |
| If this data is needed for local analysis or debugging, regenerate it in a | |
| tooling/output location that is ignored by version control. |
| teleop opr [ | ||
| [ '27', 404.3602886959084 ], | ||
| [ '695', 274.4053487090665 ], | ||
| [ '4611', 231.2546853279252 ], | ||
| [ '1507', 214.01710201592738 ], | ||
| [ '2252', 165.5464811572625 ], | ||
| [ '2399', 145.45588406885133 ], | ||
| [ '4145', 103.24468687965383 ], | ||
| [ '291', 94.21406764382665 ], | ||
| [ '8393', 88.07587771393649 ], | ||
| [ '4121', 77.85751477349575 ], | ||
| [ '2172', 73.37875033401536 ], | ||
| [ '3201', 68.53134929842241 ], | ||
| [ '1014', 55.763967929475236 ], | ||
| [ '1308', 48.678591092391 ], | ||
| [ '120', 47.34566255116512 ], | ||
| [ '2603', 42.82623554385303 ], | ||
| [ '9194', 41.164660658838834 ], | ||
| [ '10011', 40.723335160638534 ], | ||
| [ '4085', 38.219401785324436 ], | ||
| [ '10208', 36.935050458188236 ], | ||
| [ '3193', 28.137941470123636 ], | ||
| [ '3777', 28.0586920594894 ], | ||
| [ '8145', 28.05406625610777 ], | ||
| [ '8713', 27.083642004014482 ], | ||
| [ '9075', 26.17572243277031 ], | ||
| [ '5413', 24.601064819127036 ], | ||
| [ '6834', 24.190857834914713 ], | ||
| [ '10207', 22.592118319672455 ], | ||
| [ '7885', 19.21632346643769 ], | ||
| [ '6916', 18.029815073601103 ], | ||
| [ '8718', 13.223779085892573 ], | ||
| [ '9622', 12.253016708472847 ], | ||
| [ '7165', 12.200768873927787 ], | ||
| [ '8222', 3.7888140985315877 ], | ||
| [ '5811', -2.27463812367839 ], | ||
| [ '9643', -2.4777782195577416 ], | ||
| [ '6964', -4.213831135316625 ], | ||
| [ '8140', -9.568035964727759 ], | ||
| [ '9767', -13.913716581996109 ], | ||
| [ '8243', -14.576481079638917 ], | ||
| [ '6936', -36.51250167110157 ] | ||
| ] |
There was a problem hiding this comment.
This looks like local analysis output (OPR data) rather than source/config consumed by the app, and the repo has no references to oprCompare/. Please remove or relocate it (and consider adding a .gitignore rule) so it doesn’t ship accidentally.
| teleop opr [ | |
| [ '27', 404.3602886959084 ], | |
| [ '695', 274.4053487090665 ], | |
| [ '4611', 231.2546853279252 ], | |
| [ '1507', 214.01710201592738 ], | |
| [ '2252', 165.5464811572625 ], | |
| [ '2399', 145.45588406885133 ], | |
| [ '4145', 103.24468687965383 ], | |
| [ '291', 94.21406764382665 ], | |
| [ '8393', 88.07587771393649 ], | |
| [ '4121', 77.85751477349575 ], | |
| [ '2172', 73.37875033401536 ], | |
| [ '3201', 68.53134929842241 ], | |
| [ '1014', 55.763967929475236 ], | |
| [ '1308', 48.678591092391 ], | |
| [ '120', 47.34566255116512 ], | |
| [ '2603', 42.82623554385303 ], | |
| [ '9194', 41.164660658838834 ], | |
| [ '10011', 40.723335160638534 ], | |
| [ '4085', 38.219401785324436 ], | |
| [ '10208', 36.935050458188236 ], | |
| [ '3193', 28.137941470123636 ], | |
| [ '3777', 28.0586920594894 ], | |
| [ '8145', 28.05406625610777 ], | |
| [ '8713', 27.083642004014482 ], | |
| [ '9075', 26.17572243277031 ], | |
| [ '5413', 24.601064819127036 ], | |
| [ '6834', 24.190857834914713 ], | |
| [ '10207', 22.592118319672455 ], | |
| [ '7885', 19.21632346643769 ], | |
| [ '6916', 18.029815073601103 ], | |
| [ '8718', 13.223779085892573 ], | |
| [ '9622', 12.253016708472847 ], | |
| [ '7165', 12.200768873927787 ], | |
| [ '8222', 3.7888140985315877 ], | |
| [ '5811', -2.27463812367839 ], | |
| [ '9643', -2.4777782195577416 ], | |
| [ '6964', -4.213831135316625 ], | |
| [ '8140', -9.568035964727759 ], | |
| [ '9767', -13.913716581996109 ], | |
| [ '8243', -14.576481079638917 ], | |
| [ '6936', -36.51250167110157 ] | |
| ] | |
| Removed generated local OPR analysis output from version control. | |
| Regenerate this file locally if needed, but do not commit analysis artifacts under `oprCompare/`. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09354e7380
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!networkMonitor.isConnected.value) { | ||
| return@withContext fallback |
There was a problem hiding this comment.
Retry current-event fetch after monitor reports connectivity
resolveCurrentEvent bails out immediately when isConnected.value is false, but NetworkMonitor initializes this state as false and flips it asynchronously via callbacks. On cold start with working internet, this method can return the fallback event before connectivity is marked validated, and because initialization only calls it once, the session can stay pinned to the fallback event and submit pit data under the wrong event key.
Useful? React with 👍 / 👎.
| ) | ||
| val formState: StateFlow<PitFormState> = _formState.asStateFlow() | ||
|
|
||
| private var eventKey = buildFallbackEventKey() |
There was a problem hiding this comment.
Avoid mutating event key after user actions can run
The view model starts with a fallback eventKey and later overwrites it asynchronously after resolveCurrentEvent returns. Because createNewTab can be invoked during this window, tabs may be created under the fallback key and then appear to disappear once eventKey is replaced and observeTabs() starts collecting a different event stream.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/team695/scoutifyapp/data/api/NetworkMonitor.kt`:
- Line 243: The increment of the errors counter is inconsistent: replace the use
of "errors += 1" with the pre-increment form "++errors" to match the rest of the
file (see other increments at lines using ++errors); update the occurrence in
NetworkMonitor.kt where the local variable errors is incremented so it uses
++errors for consistent coding style across the file.
- Around line 258-261: The log for the "maximum errors limit exceeded for
pushing repositories" in NetworkMonitor is using Log.d but should use the same
severity as the fetch equivalent; change the Log.d call to Log.e so the push
maximum-errors-exceeded condition is logged as an error (replace the Log.d(...)
call that logs "Maximum errors limit exceeded for pushing repositories:
$maxErrors errors" with Log.e using the same tag/message), keeping the tag
"Network_Monitor" and message text identical for consistency with the fetch
error at line 189.
In
`@app/src/main/java/com/team695/scoutifyapp/data/repository/PitScoutingRepository.kt`:
- Around line 79-93: The resolved network eventId must be normalized to the
app's local key format to avoid duplicate keys; in resolveCurrentEvent(), after
calling surveyService.getCurrentEvent() (inside the runCatching/map chain)
convert the server event id using the existing formatEventId(...) function
before constructing or returning the PitCurrentEvent (or inside
toPitCurrentEvent mapping), so the returned PitCurrentEvent.eventId (and any
local-key fields) use formatEventId(serverEvent.eventId) and fall back to the
existing fallback when null.
- Around line 309-322: The reset-failure path currently only stores lastError in
the in-memory Tab but persistTab/rowToTab don't persist or expose it and the API
still reports success; update persistTab and rowToTab to serialize/deserialize
the Tab.lastError field and ensure callers can observe/reset failures (either by
returning a failure result or by logging/reporting the error). Specifically,
modify persistTab to include lastError when writing the DB row, update rowToTab
to populate Tab.lastError from the stored column, and adjust the clearTab call
site (and any method that reports success after persistTab) to propagate or
surface the reset exception (from clearTab) instead of silently treating the
operation as fully successful; keep PitScoutingStatus.SUBMITTED semantics but
ensure the local failure is persisted and visible to callers.
- Around line 140-153: The clearTab function fails to refresh the tab's
formVersion when regenerateFormId is true, leaving versionMismatch stale; update
the cleared copy to set formVersion = if (regenerateFormId)
PitFormDataProvider.CURRENT_FORM_VERSION (or
PitFormDataProvider.getCurrentFormVersion()) else existing.formVersion so that
regenerating the form resets both fields and formVersion alongside formId and
createdAt.
In `@app/src/main/java/com/team695/scoutifyapp/ui/screens/PitScoutingScreen.kt`:
- Around line 622-629: The submit Button currently enables only based on
isSubmitting; update its enabled condition to also require versionMismatch to be
false (e.g., enabled = !isSubmitting && !versionMismatch) so users cannot submit
when the tab is version-mismatched, and ensure the same guard remains inside
submitCurrentTab() to defensively block submissions from mismatched drafts;
adjust references around the Button declaration and the submitCurrentTab()
method to include versionMismatch checks.
In
`@app/src/main/java/com/team695/scoutifyapp/ui/viewModels/PitScoutingViewModel.kt`:
- Around line 38-63: The create-tab flow must be guarded until the real event
key is resolved in initializeEvent(): update createNewTab() to check the
initialization state (e.g., return early if _formState.value.isLoading is true
or if eventKey == buildFallbackEventKey()) so taps during the async init cannot
persist drafts under the fallback event; locate createNewTab(), add the guard at
its start, and rely on the existing initializeEvent()/observeTabs() flow (and
starterTabCreated) to allow creation only after loading completes.
In `@oprCompare/neg.txt`:
- Around line 1-43: The files neg.txt and pos.txt are misrepresenting their
contents (neg.txt has 37 positive and 4 negative values, pos.txt contains
negatives), so either filter or rename them to match their semantic contract;
update the data generation or preprocessing that writes 'neg.txt' and 'pos.txt'
so that 'neg.txt' contains only values < 0 and 'pos.txt' contains only values >
0 (or alternatively rename the files to indicate mixed-sign data), and ensure
any downstream code that reads these files uses the corrected filenames or the
filtered datasets.
In `@oprCompare/pos.txt`:
- Around line 1-43: pos.txt contains negative-valued entries (e.g., '5811'
-2.486..., '9643' -2.590..., etc.) but is named/treated as a positive-only
dataset; either filter out negatives when producing pos.txt or rename it and
update any consumers. Fix option A: ensure the producer or preprocessing step
only writes entries with value > 0 into pos.txt (filter by numeric value > 0
before writing). Fix option B: rename pos.txt to something like
mixed_sign_positions and update any code that loads pos.txt (comparison logic,
sign-based branches) to handle mixed signs. Locate the writer/loader that
emits/reads pos.txt and apply the chosen change so filenames and dataset
semantics match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ebca0ba-afd6-481a-a2b0-1c22b9d630a6
📒 Files selected for processing (15)
app/src/main/java/com/team695/scoutifyapp/MainActivity.ktapp/src/main/java/com/team695/scoutifyapp/data/api/NetworkMonitor.ktapp/src/main/java/com/team695/scoutifyapp/data/api/service/SurveyService.ktapp/src/main/java/com/team695/scoutifyapp/data/repository/GameDetailRepository.ktapp/src/main/java/com/team695/scoutifyapp/data/repository/PitFormDataProvider.ktapp/src/main/java/com/team695/scoutifyapp/data/repository/PitScoutingRepository.ktapp/src/main/java/com/team695/scoutifyapp/data/types/PitScoutingTab.ktapp/src/main/java/com/team695/scoutifyapp/data/update/UpdateReceiver.ktapp/src/main/java/com/team695/scoutifyapp/ui/components/form/DynamicFormField.ktapp/src/main/java/com/team695/scoutifyapp/ui/components/form/FormComponents.ktapp/src/main/java/com/team695/scoutifyapp/ui/screens/PitScoutingScreen.ktapp/src/main/java/com/team695/scoutifyapp/ui/viewModels/PitScoutingViewModel.ktoprCompare/1.txtoprCompare/neg.txtoprCompare/pos.txt
| } | ||
| else { | ||
| } else { | ||
| errors += 1 |
There was a problem hiding this comment.
Standardize error increment for consistency.
This line still uses errors += 1 while the rest of the file was updated to use ++errors (lines 131, 170, 210). This inconsistency should be addressed to maintain uniform coding style throughout the refactoring.
🔧 Proposed fix
if (result.isSuccess) {
iter.remove()
} else {
- errors += 1
+ ++errors
val exception = result.exceptionOrNull()
Log.e("Network_Monitor", "Error pushing repo", exception)
}📝 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.
| errors += 1 | |
| if (result.isSuccess) { | |
| iter.remove() | |
| } else { | |
| +errors | |
| val exception = result.exceptionOrNull() | |
| Log.e("Network_Monitor", "Error pushing repo", exception) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/team695/scoutifyapp/data/api/NetworkMonitor.kt` at line
243, The increment of the errors counter is inconsistent: replace the use of
"errors += 1" with the pre-increment form "++errors" to match the rest of the
file (see other increments at lines using ++errors); update the occurrence in
NetworkMonitor.kt where the local variable errors is incremented so it uses
++errors for consistent coding style across the file.
| Log.d( | ||
| "Network_Monitor", | ||
| "Maximum errors limit exceeded for pushing repositories: $maxErrors errors" | ||
| ) |
There was a problem hiding this comment.
Use consistent log level for error conditions.
This maximum-errors-exceeded condition uses Log.d (debug), while the equivalent condition for fetch operations uses Log.e (error) at line 189. Both represent failure scenarios and should be logged at the same severity level for consistency and proper monitoring.
📝 Proposed fix
if (pushList.isEmpty()) {
Log.d("Network_Monitor", "Pushed all data successfully!")
} else {
- Log.d(
+ Log.e(
"Network_Monitor",
"Maximum errors limit exceeded for pushing repositories: $maxErrors errors"
)
}📝 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.
| Log.d( | |
| "Network_Monitor", | |
| "Maximum errors limit exceeded for pushing repositories: $maxErrors errors" | |
| ) | |
| Log.e( | |
| "Network_Monitor", | |
| "Maximum errors limit exceeded for pushing repositories: $maxErrors errors" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/team695/scoutifyapp/data/api/NetworkMonitor.kt` around
lines 258 - 261, The log for the "maximum errors limit exceeded for pushing
repositories" in NetworkMonitor is using Log.d but should use the same severity
as the fetch equivalent; change the Log.d call to Log.e so the push
maximum-errors-exceeded condition is logged as an error (replace the Log.d(...)
call that logs "Maximum errors limit exceeded for pushing repositories:
$maxErrors errors" with Log.e using the same tag/message), keeping the tag
"Network_Monitor" and message text identical for consistency with the fetch
error at line 189.
| suspend fun resolveCurrentEvent(defaultEventKey: String): PitCurrentEvent { | ||
| return withContext(Dispatchers.IO) { | ||
| val fallback = PitCurrentEvent( | ||
| eventId = defaultEventKey, | ||
| eventDisplayName = defaultEventKey | ||
| ) | ||
|
|
||
| if (!networkMonitor.isConnected.value) { | ||
| return@withContext fallback | ||
| } | ||
|
|
||
| runCatching { surveyService.getCurrentEvent() } | ||
| .map { response -> response.toPitCurrentEvent() ?: fallback } | ||
| .getOrDefault(fallback) | ||
| } |
There was a problem hiding this comment.
Normalize the network event id into the app's local key format.
This repository stores tabs under YYYY_CODE keys and only converts them to API format through formatEventId(). Returning the raw server eventId here risks splitting the same event across two local keys, e.g. 2026_MNWI vs 2026mnwi, which makes existing drafts disappear as soon as the app switches from fallback to network-resolved events.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/team695/scoutifyapp/data/repository/PitScoutingRepository.kt`
around lines 79 - 93, The resolved network eventId must be normalized to the
app's local key format to avoid duplicate keys; in resolveCurrentEvent(), after
calling surveyService.getCurrentEvent() (inside the runCatching/map chain)
convert the server event id using the existing formatEventId(...) function
before constructing or returning the PitCurrentEvent (or inside
toPitCurrentEvent mapping), so the returned PitCurrentEvent.eventId (and any
local-key fields) use formatEventId(serverEvent.eventId) and fall back to the
existing fallback when null.
| suspend fun clearTab(tabId: String, regenerateFormId: Boolean = false): PitScoutingTab? { | ||
| return withContext(Dispatchers.IO) { | ||
| val existing = getTabById(tabId) ?: return@withContext null | ||
| val cleared = existing.copy( | ||
| teamNumber = "", | ||
| tabName = buildFallbackTabName(existing.tabId), | ||
| formId = if (regenerateFormId) UUID.randomUUID().toString() else existing.formId, | ||
| fields = PitFormDataProvider.createEmptyFormFields(), | ||
| images = PitImageBundle(), | ||
| isDraft = true, | ||
| isSubmitted = false, | ||
| submissionTime = null, | ||
| createdAt = if (regenerateFormId) nowIso() else existing.createdAt, | ||
| updatedAt = nowIso(), |
There was a problem hiding this comment.
Refresh formVersion when regenerating the form.
clearTab() rebuilds fields from the current schema but keeps the previous formVersion. Clearing an outdated tab can therefore leave versionMismatch stuck on even though the field list was already reset.
Proposed fix
val cleared = existing.copy(
teamNumber = "",
tabName = buildFallbackTabName(existing.tabId),
formId = if (regenerateFormId) UUID.randomUUID().toString() else existing.formId,
+ formVersion = PitFormDataProvider.FORM_VERSION,
fields = PitFormDataProvider.createEmptyFormFields(),
images = PitImageBundle(),
isDraft = true,
isSubmitted = false,📝 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.
| suspend fun clearTab(tabId: String, regenerateFormId: Boolean = false): PitScoutingTab? { | |
| return withContext(Dispatchers.IO) { | |
| val existing = getTabById(tabId) ?: return@withContext null | |
| val cleared = existing.copy( | |
| teamNumber = "", | |
| tabName = buildFallbackTabName(existing.tabId), | |
| formId = if (regenerateFormId) UUID.randomUUID().toString() else existing.formId, | |
| fields = PitFormDataProvider.createEmptyFormFields(), | |
| images = PitImageBundle(), | |
| isDraft = true, | |
| isSubmitted = false, | |
| submissionTime = null, | |
| createdAt = if (regenerateFormId) nowIso() else existing.createdAt, | |
| updatedAt = nowIso(), | |
| suspend fun clearTab(tabId: String, regenerateFormId: Boolean = false): PitScoutingTab? { | |
| return withContext(Dispatchers.IO) { | |
| val existing = getTabById(tabId) ?: return@withContext null | |
| val cleared = existing.copy( | |
| teamNumber = "", | |
| tabName = buildFallbackTabName(existing.tabId), | |
| formId = if (regenerateFormId) UUID.randomUUID().toString() else existing.formId, | |
| formVersion = PitFormDataProvider.FORM_VERSION, | |
| fields = PitFormDataProvider.createEmptyFormFields(), | |
| images = PitImageBundle(), | |
| isDraft = true, | |
| isSubmitted = false, | |
| submissionTime = null, | |
| createdAt = if (regenerateFormId) nowIso() else existing.createdAt, | |
| updatedAt = nowIso(), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/team695/scoutifyapp/data/repository/PitScoutingRepository.kt`
around lines 140 - 153, The clearTab function fails to refresh the tab's
formVersion when regenerateFormId is true, leaving versionMismatch stale; update
the cleared copy to set formVersion = if (regenerateFormId)
PitFormDataProvider.CURRENT_FORM_VERSION (or
PitFormDataProvider.getCurrentFormVersion()) else existing.formVersion so that
regenerating the form resets both fields and formVersion alongside formId and
createdAt.
| runCatching { | ||
| clearTab(preparedTab.tabId, regenerateFormId = true) | ||
| }.onFailure { resetError -> | ||
| persistTab( | ||
| preparedTab.copy( | ||
| isDraft = false, | ||
| isSubmitted = true, | ||
| submissionTime = nowIso(), | ||
| updatedAt = nowIso(), | ||
| syncStatus = PitScoutingStatus.SUBMITTED, | ||
| lastError = "Submitted remotely, but local tab reset failed: ${resetError.message.orEmpty()}" | ||
| ) | ||
| ) | ||
| ) | ||
| } |
There was a problem hiding this comment.
The reset-failure path is effectively silent right now.
This branch writes lastError, but persistTab()/rowToTab() never serialize that field, and the method still reports success. If clearTab(..., regenerateFormId = true) fails, the caller cannot distinguish “submitted and cleared” from “submitted but left in a broken local state.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/team695/scoutifyapp/data/repository/PitScoutingRepository.kt`
around lines 309 - 322, The reset-failure path currently only stores lastError
in the in-memory Tab but persistTab/rowToTab don't persist or expose it and the
API still reports success; update persistTab and rowToTab to
serialize/deserialize the Tab.lastError field and ensure callers can
observe/reset failures (either by returning a failure result or by
logging/reporting the error). Specifically, modify persistTab to include
lastError when writing the DB row, update rowToTab to populate Tab.lastError
from the stored column, and adjust the clearTab call site (and any method that
reports success after persistTab) to propagate or surface the reset exception
(from clearTab) instead of silently treating the operation as fully successful;
keep PitScoutingStatus.SUBMITTED semantics but ensure the local failure is
persisted and visible to callers.
| Button( | ||
| onClick = onSubmit, | ||
| modifier = Modifier.weight(1f), | ||
| enabled = !isSubmitting, | ||
| shape = RoundedCornerShape(14.dp), | ||
| colors = ButtonDefaults.buttonColors(containerColor = Color(0xFF153A54), contentColor = Color.White), | ||
| contentPadding = PaddingValues(vertical = 9.dp) | ||
| ) { |
There was a problem hiding this comment.
Prevent submit on version-mismatched tabs.
Line 625 only checks isSubmitting. After this schema change, an older draft still validates against its stale fields list and can be uploaded without the new required intake answer. Please gate submit on versionMismatch as well, and keep the same guard in submitCurrentTab().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/team695/scoutifyapp/ui/screens/PitScoutingScreen.kt`
around lines 622 - 629, The submit Button currently enables only based on
isSubmitting; update its enabled condition to also require versionMismatch to be
false (e.g., enabled = !isSubmitting && !versionMismatch) so users cannot submit
when the tab is version-mismatched, and ensure the same guard remains inside
submitCurrentTab() to defensively block submissions from mismatched drafts;
adjust references around the Button declaration and the submitCurrentTab()
method to include versionMismatch checks.
| private var eventKey = buildFallbackEventKey() | ||
| private var selectedTabId: String? = null | ||
| private var teamsJob: Job? = null | ||
| private var tabsJob: Job? = null | ||
| private var starterTabCreated = false | ||
|
|
||
| init { | ||
| observeTabs() | ||
| refreshSupportingData() | ||
| initializeEvent() | ||
| } | ||
|
|
||
| private fun observeTabs() { | ||
| private fun initializeEvent() { | ||
| viewModelScope.launch { | ||
| val currentEvent = repository.resolveCurrentEvent(eventKey) | ||
| eventKey = currentEvent.eventId | ||
| starterTabCreated = false | ||
| _formState.update { state -> | ||
| state.copy( | ||
| eventId = currentEvent.eventId, | ||
| eventDisplayName = currentEvent.eventDisplayName, | ||
| isLoading = true, | ||
| error = null | ||
| ) | ||
| } | ||
| observeTabs() | ||
| refreshSupportingData() | ||
| } |
There was a problem hiding this comment.
Block tab creation until the event key is resolved.
Line 38 seeds eventKey with the fallback value, and this async init swaps it later. Since the create-tab flows remain reachable while isLoading is true, a quick tap can persist a draft under the fallback event and then hide it when observeTabs() starts collecting the resolved event instead. Guard createNewTab() until initialization finishes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/team695/scoutifyapp/ui/viewModels/PitScoutingViewModel.kt`
around lines 38 - 63, The create-tab flow must be guarded until the real event
key is resolved in initializeEvent(): update createNewTab() to check the
initialization state (e.g., return early if _formState.value.isLoading is true
or if eventKey == buildFallbackEventKey()) so taps during the async init cannot
persist drafts under the fallback event; locate createNewTab(), add the guard at
its start, and rely on the existing initializeEvent()/observeTabs() flow (and
starterTabCreated) to allow creation only after loading completes.
| teleop opr [ | ||
| [ '27', 413.06453750499384 ], | ||
| [ '695', 251.62570461409706 ], | ||
| [ '4611', 202.43437681242438 ], | ||
| [ '1507', 192.2513450745422 ], | ||
| [ '2252', 154.24880109997412 ], | ||
| [ '2399', 120.53981061781971 ], | ||
| [ '291', 80.46484678513568 ], | ||
| [ '8393', 77.95611587991105 ], | ||
| [ '4145', 74.12483493126946 ], | ||
| [ '2172', 62.46166905455011 ], | ||
| [ '4121', 58.58030085783609 ], | ||
| [ '3201', 52.92715412833798 ], | ||
| [ '1014', 44.279957336737105 ], | ||
| [ '120', 38.22604778454931 ], | ||
| [ '2603', 36.84037485323583 ], | ||
| [ '9194', 34.05662426161272 ], | ||
| [ '10011', 33.90606369805352 ], | ||
| [ '1308', 32.13065956137563 ], | ||
| [ '10208', 31.506273445326553 ], | ||
| [ '4085', 25.52763290846716 ], | ||
| [ '10207', 25.42282477468923 ], | ||
| [ '8713', 23.776595974606266 ], | ||
| [ '5413', 22.070825038797388 ], | ||
| [ '8145', 21.23968200414648 ], | ||
| [ '6834', 18.456879133501218 ], | ||
| [ '3193', 18.23750944898354 ], | ||
| [ '6916', 17.202899737665998 ], | ||
| [ '9075', 16.771082994684875 ], | ||
| [ '3777', 16.566839123373807 ], | ||
| [ '7165', 15.942791599036582 ], | ||
| [ '7885', 15.7747064306565 ], | ||
| [ '6964', 14.167909041582178 ], | ||
| [ '9622', 13.946543687666436 ], | ||
| [ '8718', 10.009164410732938 ], | ||
| [ '9643', 6.834590380162182 ], | ||
| [ '5811', 6.3047146444808 ], | ||
| [ '8222', 4.367674231405969 ], | ||
| [ '8243', -0.08907799347307964 ], | ||
| [ '9767', -0.3216293011037005 ], | ||
| [ '8140', -1.76333356532534 ], | ||
| [ '6936', -3.963002021656522 ] | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify sign distribution in oprCompare/neg.txt and oprCompare/pos.txt
python - <<'PY'
import re, pathlib
for p in [pathlib.Path("oprCompare/neg.txt"), pathlib.Path("oprCompare/pos.txt")]:
nums = [float(x) for x in re.findall(r",\s*(-?\d+(?:\.\d+)?)\s*\]", p.read_text())]
pos = sum(1 for n in nums if n > 0)
neg = sum(1 for n in nums if n < 0)
zero = sum(1 for n in nums if n == 0)
print(f"{p}: total={len(nums)} pos={pos} neg={neg} zero={zero}")
PYRepository: 1834423612/Scoutify-Android-App
Length of output: 172
neg.txt and pos.txt filenames do not reflect their actual data distribution.
neg.txt contains 37 positive and only 4 negative values (90% positive). Additionally, pos.txt has 7 negative values compared to neg.txt's 4, suggesting the files are either reversed, incorrectly filtered, or misnamed. If downstream code partitions data by filename semantics, this will produce incorrect comparisons.
Either filter each file to match its name contract (neg.txt → values < 0, pos.txt → values > 0) or rename to reflect mixed-sign data.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@oprCompare/neg.txt` around lines 1 - 43, The files neg.txt and pos.txt are
misrepresenting their contents (neg.txt has 37 positive and 4 negative values,
pos.txt contains negatives), so either filter or rename them to match their
semantic contract; update the data generation or preprocessing that writes
'neg.txt' and 'pos.txt' so that 'neg.txt' contains only values < 0 and 'pos.txt'
contains only values > 0 (or alternatively rename the files to indicate
mixed-sign data), and ensure any downstream code that reads these files uses the
corrected filenames or the filtered datasets.
| teleop opr [ | ||
| [ '27', 404.6980666302104 ], | ||
| [ '695', 274.3967463336902 ], | ||
| [ '4611', 230.9801945503192 ], | ||
| [ '1507', 214.63979832431855 ], | ||
| [ '2252', 165.5752807000386 ], | ||
| [ '2399', 146.1376193867555 ], | ||
| [ '4145', 103.14842918483612 ], | ||
| [ '291', 94.86535592988169 ], | ||
| [ '8393', 88.34162056988467 ], | ||
| [ '4121', 78.41783831801162 ], | ||
| [ '2172', 71.63615312413508 ], | ||
| [ '3201', 68.30839246348101 ], | ||
| [ '1014', 56.02686101212037 ], | ||
| [ '1308', 49.376652370655464 ], | ||
| [ '120', 47.10443949612512 ], | ||
| [ '2603', 43.25649554618409 ], | ||
| [ '9194', 41.25857657248875 ], | ||
| [ '10011', 40.99149463314703 ], | ||
| [ '4085', 37.68161859744629 ], | ||
| [ '10208', 37.28306763851922 ], | ||
| [ '3193', 28.641259900393553 ], | ||
| [ '3777', 28.083645940249834 ], | ||
| [ '8145', 27.978749475128634 ], | ||
| [ '9075', 26.47267878821969 ], | ||
| [ '5413', 24.77463517204321 ], | ||
| [ '8713', 24.495914921571853 ], | ||
| [ '6834', 24.216845974614564 ], | ||
| [ '10207', 22.339558150758908 ], | ||
| [ '7885', 19.390665976869965 ], | ||
| [ '6916', 19.06761127665281 ], | ||
| [ '8718', 13.410902991207555 ], | ||
| [ '9622', 12.194385924705738 ], | ||
| [ '7165', 12.009197253150488 ], | ||
| [ '8222', 3.7109017731074805 ], | ||
| [ '5811', -2.486279802327215 ], | ||
| [ '9643', -2.590359072712503 ], | ||
| [ '6964', -4.243361422845558 ], | ||
| [ '8140', -11.562362054815253 ], | ||
| [ '8243', -14.179575023437806 ], | ||
| [ '9767', -16.913952451235996 ], | ||
| [ '6936', -36.602373625644944 ] | ||
| ] |
There was a problem hiding this comment.
pos.txt also includes negative entries.
Line 36 through Line 42 are negative values, which conflicts with a “positive-only” dataset interpretation and can break sign-based compare logic.
Align file semantics with content: either keep only value > 0 in pos.txt or rename files to indicate mixed-sign datasets.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@oprCompare/pos.txt` around lines 1 - 43, pos.txt contains negative-valued
entries (e.g., '5811' -2.486..., '9643' -2.590..., etc.) but is named/treated as
a positive-only dataset; either filter out negatives when producing pos.txt or
rename it and update any consumers. Fix option A: ensure the producer or
preprocessing step only writes entries with value > 0 into pos.txt (filter by
numeric value > 0 before writing). Fix option B: rename pos.txt to something
like mixed_sign_positions and update any code that loads pos.txt (comparison
logic, sign-based branches) to handle mixed signs. Locate the writer/loader that
emits/reads pos.txt and apply the chosen change so filenames and dataset
semantics match.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores