ADFA-934: Build output timestamp - #1586
Open
dara-abijo-adfa wants to merge 19 commits into
Open
Conversation
dara-abijo-adfa
requested review from
fryanpan,
hal-eisen-adfa,
itsaky-adfa,
jatezzz and
jomen-adfa
July 27, 2026 23:24
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Contributor
📝 Walkthrough
WalkthroughBuild output now shows timestamp and delta prefixes on streamed lines, computed in ChangesBuild output controls
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant EditorBuildEventListener
participant BuildOutputViewModel
participant BuildOutputFragment
participant EditorBottomSheet
EditorBuildEventListener->>EditorBuildEventListener: Reset build timers on prepareBuild
EditorBuildEventListener->>BuildOutputViewModel: Format output line with timing prefix
BuildOutputViewModel-->>EditorBuildEventListener: Return prefixed line
EditorBuildEventListener->>BuildOutputFragment: Append prefixed output to session
EditorBottomSheet->>BuildOutputFragment: Invoke showViewOptions on click
BuildOutputFragment->>BuildOutputFragment: Display popup with checkbox toggles
BuildOutputFragment->>BuildOutputViewModel: Update showTimestamps / showDeltas / showLineNumbers
BuildOutputViewModel-->>BuildOutputFragment: Emit updated visibility state
BuildOutputFragment->>BuildOutputViewModel: filterLines with current toggles
BuildOutputViewModel-->>BuildOutputFragment: Return visible text
BuildOutputFragment->>BuildOutputFragment: Render text if editor generation is current
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![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: 4
🧹 Nitpick comments (2)
app/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.kt (1)
183-197: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding unit test coverage for the new timing/formatting logic.
formatOutputcontains non-trivial timing math (total/step deltas, blank-line/trailing-newline preservation) but no accompanying test file was included in this PR. As per coding guidelines, new/changed non-UI logic should target at least 50% line and branch coverage.Based on learnings and coding guidelines: "Use unit tests for non-UI logic, cover error and edge paths, and target at least 50% line and branch coverage for new or changed non-UI code." Want me to draft a JUnit test class for
formatOutput/resetBuildTimers?🤖 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 `@app/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.kt` around lines 183 - 197, Add unit tests for the non-UI formatting logic in formatOutput and timer behavior in resetBuildTimers. Cover total and step timing prefixes, multi-line output, blank lines, inputs with and without trailing newlines, and timer reset behavior, targeting at least 50% line and branch coverage.Source: Coding guidelines
app/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.kt (1)
69-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider wiring long-press tooltip help for the new toggle chips.
The three new chips (line numbers, timestamps, deltas) are new interactive elements with no evident long-press help wiring (e.g. via the
TooltipTag-based mechanism used elsewhere, such aseditor?.tag = TooltipTag.PROJECT_BUILD_OUTPUTinBuildOutputFragment.kt).Based on coding guidelines: "New interactive elements and screens must provide long-press contextual help through idetooltips and the three-tier tooltip system." Please confirm whether an existing mechanism (not shown in this diff) already covers these chips, or whether tooltip tagging should be added here.
🤖 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 `@app/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.kt` around lines 69 - 92, Verify whether chipLineNumbers, chipTimestamps, and chipDeltas already receive long-press help through an existing tooltip mechanism; if not, add the appropriate TooltipTag-based three-tier tooltip wiring for each chip in the controller’s setup flow, using distinct contextual tags and preserving the existing visibility, checked-state, and listener behavior.Source: Coding guidelines
🤖 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
`@app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt`:
- Around line 88-99: Separate the generation used for session-file persistence
from editorContentGeneration. Add a persistence generation that is incremented
only by clearOutput(), capture and propagate both generations through
processLogs and flushToEditor, and use the persistence generation to gate
buildOutputViewModel.append(text) while retaining editorContentGeneration for
visible appendBatch calls. Ensure filter and display-toggle re-renders cannot
discard drained output.
In
`@app/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.kt`:
- Around line 79-104: Update prepareBuild and onProgressEvent to capture the
result of checkActivity(...) in a local activity instance and return when it is
null, then use that captured instance throughout each method instead of
re-reading the throwing activity accessor. Preserve the existing build-status,
timer, output, and progress behavior while eliminating the
time-of-check/time-of-use race.
- Around line 45-46: Update build elapsed-time tracking in
EditorBuildEventListener to use SystemClock.elapsedRealtime() for
buildStartTimeMs, lastOutputTimeMs, and all totalDeltaMs/stepDeltaMs
calculations, including the referenced later sections. Preserve
System.currentTimeMillis()/Instant only for displayed wall-clock timestamps,
ensuring duration values remain monotonic and non-negative.
In `@app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt`:
- Around line 202-227: Update PREFIX_TIME_FORMAT used by formatLinePrefix to
apply an explicit ASCII DecimalStyle, ensuring the [HH:mm:ss.SSS] timestamp
always emits ASCII digits regardless of the device locale. Do not rely solely on
Locale.US; preserve the existing timestamp pattern and formatting behavior.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.kt`:
- Around line 69-92: Verify whether chipLineNumbers, chipTimestamps, and
chipDeltas already receive long-press help through an existing tooltip
mechanism; if not, add the appropriate TooltipTag-based three-tier tooltip
wiring for each chip in the controller’s setup flow, using distinct contextual
tags and preserving the existing visibility, checked-state, and listener
behavior.
In
`@app/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.kt`:
- Around line 183-197: Add unit tests for the non-UI formatting logic in
formatOutput and timer behavior in resetBuildTimers. Cover total and step timing
prefixes, multi-line output, blank lines, inputs with and without trailing
newlines, and timer reset behavior, targeting at least 50% line and branch
coverage.
🪄 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 Plus
Run ID: d78f0844-2eb6-4697-a35e-eed2f14c5974
📒 Files selected for processing (7)
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.ktapp/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.ktapp/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.ktapp/src/main/res/layout/layout_log_filter_bar.xmlapp/src/test/java/com/itsaky/androidide/viewmodel/BuildOutputFilterTest.ktresources/src/main/res/values/strings.xml
jatezzz
approved these changes
Jul 29, 2026
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt (1)
249-251: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the complete trailing line separator before splitting.
lineSequence()treats CRLF as one separator, butfilterLines()only strips a final\n. When content ends with\r\n, no separator remains to trigger the existing blank-line drop, sofilterLines()can append an extra blank line for that line ending. Strip\r\nbefore stripping isolated\nor\r, and add a CRLF regression test.🤖 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 `@app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt` around lines 249 - 251, Update the body preparation near `filterLines()` to remove one complete trailing CRLF separator before handling isolated trailing `\n` or `\r`, preserving the existing blank-line behavior for newline-terminated content. Add a regression test covering CRLF-terminated input and verifying that `filterLines()` does not append an extra blank line.
🤖 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.
Outside diff comments:
In `@app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt`:
- Around line 249-251: Update the body preparation near `filterLines()` to
remove one complete trailing CRLF separator before handling isolated trailing
`\n` or `\r`, preserving the existing blank-line behavior for newline-terminated
content. Add a regression test covering CRLF-terminated input and verifying that
`filterLines()` does not append an extra blank line.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6bbaad46-d8b5-4303-a44e-e821f4a61c9d
📒 Files selected for processing (6)
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.ktapp/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.ktapp/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.ktapp/src/test/java/com/itsaky/androidide/viewmodel/BuildOutputFilterTest.ktresources/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (5)
- resources/src/main/res/values/strings.xml
- app/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.kt
- app/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.kt
- app/src/test/java/com/itsaky/androidide/viewmodel/BuildOutputFilterTest.kt
- app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@app/src/main/res/drawable/ic_tune.xml`:
- Around line 1-11: Move the drawable resource represented by ic_tune.xml from
the app module into the resources module’s drawable directory, then remove the
original app-module copy. Preserve the vector contents and resource name so
existing consumers continue resolving it through the resources module.
🪄 Autofix
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 Plus
Run ID: 579ef83a-41df-4837-860e-dfbd8739fcfe
📒 Files selected for processing (8)
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.ktapp/src/main/java/com/itsaky/androidide/fragments/output/ViewOptionsOutputFragment.ktapp/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.ktapp/src/main/res/drawable/ic_tune.xmlapp/src/main/res/layout/layout_editor_bottom_sheet.xmlidetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.ktresources/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- resources/src/main/res/values/strings.xml
- app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt
Comment on lines
+1
to
+11
| <vector | ||
| xmlns:android="http://schemas.android.com/apk/res/android" | ||
| android:width="24dp" | ||
| android:height="24dp" | ||
| android:viewportWidth="24" | ||
| android:viewportHeight="24" | ||
| android:tint="?attr/colorControlNormal"> | ||
| <path | ||
| android:fillColor="@android:color/white" | ||
| android:pathData="M3,17v2h6v-2H3zM3,5v2h10V5H3zM13,21v-2h8v-2h-8v-2h-2v6H13zM7,9v2H3v2h4v2h2V9H7zM21,13v-2H11v2H21zM15,9h2V7h4V5h-4V3h-2V9z" /> | ||
| </vector> |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Move ic_tune.xml to the resources module.
Place this drawable in resources/src/main/res/drawable/ and remove the app-module copy. The app module already consumes resources from that module. Based on learnings: drawable resources should reside in resources/src/main/res/drawable/.
🤖 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 `@app/src/main/res/drawable/ic_tune.xml` around lines 1 - 11, Move the drawable
resource represented by ic_tune.xml from the app module into the resources
module’s drawable directory, then remove the original app-module copy. Preserve
the vector contents and resource name so existing consumers continue resolving
it through the resources module.
Source: Learnings
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
3 participants
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.
Add time stamps, time deltas, and line numbers to the Build Output