Display repository workflow events to console - #14
Merged
Conversation
- The unique color for each run number should help with quickly identifying which run an event is part of.
Contributor
📝 WalkthroughWalkthroughReplaces single-run execution with a continuous polling loop, adds persistent per-repository ToolState, introduces diffing to produce typed Event objects, prints via CLIPrinter, performs parallel job fetching, adds Env/Storage helpers, new domain records/enums, JANSI dependency, and CI release packaging changes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java (1)
74-77: Narrow the exception catch or handleInterruptedExceptionseparately.Catching the generic
ExceptionmasksInterruptedException, which requires special handling (restoring the interrupt flag). Consider catching specific exceptions or adding a dedicated catch block forInterruptedException.Based on learnings, the current approach correctly avoids exposing stack traces to users.
🤖 Fix all issues with AI Agents
In @src/main/java/com/example/github_workflow_tool/api/JobClient.java:
- Around line 73-75: The catch for ExecutionException in JobClient currently
throws APIException using e.getMessage(), which hides the underlying cause;
change the handler to extract e.getCause() and use the cause's message and
throwable when constructing the APIException (e.g., pass cause.getMessage() and
the cause itself, or fall back to the ExecutionException if cause is null) so
callers see the real underlying error; update the catch block at the
ExecutionException handler in JobClient to use e.getCause() accordingly.
- Around line 55-61: The concurrent requests use CompletableFuture.supplyAsync
without a dedicated executor; add a private ExecutorService field (e.g., in
JobClient) with a fixed thread pool, change CompletableFuture.supplyAsync(() ->
getResponse(request)) to CompletableFuture.supplyAsync(() ->
getResponse(request), executor) where futures is created, and add lifecycle
management (shutdown/awaitTermination) for that ExecutorService—implement a
close/shutdown method or make JobClient AutoCloseable to ensure the thread pool
is properly terminated.
- Around line 65-79: The stream mapping can throw a NullPointerException because
response.body() may be null; update the pipeline in JobClient so after obtaining
each HttpResponse (from futures.stream().map(...)) you null-check the response
and its body before calling jsonService.parseJobResponse; either filter out
responses with null bodies (e.g., .filter(resp -> resp != null && resp.body() !=
null)) or change the mapping to handle null bodies (e.g., pass an Optional or
skip parsing and return null) and ensure APIException handling (in the
future.get() catch blocks inside the map) remains intact so
interrupted/execution errors still throw APIException with the original message.
In @src/main/java/com/example/github_workflow_tool/api/WorkflowService.java:
- Around line 32-36: In WorkflowService, guarding the inner loop against a
missing run is needed: when iterating jobResponses -> response.jobs() use a
null-safe lookup for result.get(job.runId()) (or
result.computeIfAbsent(job.runId(), ...) to create a placeholder run) before
calling .jobs().put(); if the lookup returns null, either log a warning and skip
that job or create and insert an empty Run container with a jobs map so
.jobs().put(job.id(), job) is safe.
- Around line 56-65: The askForAdditionalRunData method currently calls
jobClient.fetchData(runIds) which throws checked APIException and CLIException,
so update the askForAdditionalRunData signature to declare "throws APIException,
CLIException" (mirroring queryApi's signature) and do not swallow those
exceptions; keep the body unchanged and let callers handle them. Ensure
references: method askForAdditionalRunData, jobClient.fetchData, exceptions
APIException and CLIException, and queryApi for consistency.
In @src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.java:
- Around line 80-82: The stripAnsi method currently calls replaceAll on
ansiString without checking for null; update the stripAnsi(String ansiString)
implementation to first validate ansiString (e.g., if ansiString == null) and
return a safe value (for example, an empty string or the original null as your
convention dictates), otherwise proceed to call
ansiString.replaceAll(ANSI_REGEX, ""); reference the stripAnsi method and
ANSI_REGEX constant when making the change.
- Around line 74-78: The prettyPrintOnSeparateLines method in CLIPrinter lacks
null checks: validate that the events parameter is not null (return empty string
or appropriate default if it is) and filter out null entries before mapping;
update the stream to filter Objects::nonNull and then map Event::prettyPrint so
null list or null elements do not cause NPEs.
- Around line 67-72: The formatName method (CLIPrinter.formatName) can NPE on
name.length(); add a null check at the start of the method to handle null names
(e.g., if name == null return an empty string or other safe placeholder), then
keep the existing overflow logic that uses MAX_NAME_LENGTH and TEXT_OVERFLOW so
substring is only called when name is non-null and longer than the limit.
- Around line 55-57: The formatTag(String tag, int length) method lacks null
validation for tag; add a null check at the start of formatTag (e.g., use
Objects.requireNonNull(tag, "tag must not be null") or an explicit if (tag ==
null) throw new IllegalArgumentException("tag must not be null")) so a clear,
descriptive exception is raised instead of letting String.format produce a
NullPointerException; keep the existing padding logic intact after the check.
- Around line 63-65: The formatCommitSha method should guard against null and
short inputs to avoid NullPointerException; modify formatCommitSha to first
check if commitSha is null and return an empty string (or a configurable
fallback), then check if commitSha.length() is less than COMMIT_SHA_LENGTH and
return commitSha as-is in that case, otherwise return commitSha.substring(0,
COMMIT_SHA_LENGTH); keep references to formatCommitSha and COMMIT_SHA_LENGTH so
you update the same method.
- Around line 26-30: The DateTimeFormatter is being recreated on every call in
CLIPrinter.formatTimestamp; move it to a private static final field (e.g.,
TIMESTAMP_FORMATTER) initialized with
DateTimeFormatter.ofPattern(TIMESTAMP_FORMAT).withZone(ZoneId.systemDefault())
and update formatTimestamp(Instant) to use that static field instead of creating
a new formatter locally to avoid repeated allocations and ensure thread-safe
reuse.
- Around line 88-92: The method getPaddingBeforeBranch in CLIPrinter uses
branchName.length() without null checks; add validation at the start of
getPaddingBeforeBranch to handle a null branchName (e.g., treat null as an empty
string or throw a clear IllegalArgumentException), then use the validated value
in the Math.min(13, branchName.length()) calculation so the rest of the logic
(budgetLength and getRepeatedString) cannot NPE.
- Around line 32-53: The formatRunId method can divide by zero when average is
0; guard the normalizationRatio calculation in formatRunId by checking if
average == 0.0f and using a safe fallback (for example set normalizationRatio =
1.0f or choose a fixed contrast value) before computing r/g/b so you never
divide by zero; update the normalizationRatio assignment and keep the subsequent
clamping with Math.min to produce valid RGB values.
- Around line 84-86: Validate inputs in getRepeatedString: check that pattern is
not null and length is not negative; if pattern is null or length < 0, throw an
IllegalArgumentException with a clear message. After validation, you can keep
the current implementation or use pattern.repeat(length) for clarity; also
update the method Javadoc to document the parameter constraints and thrown
exception.
In @src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:
- Around line 97-105: runAfter.startedAt() can be null for waiting/requested
runs, so creating a WorkflowQueuedEvent with a null timestamp breaks
sorting/display; update the code that constructs the WorkflowQueuedEvent to use
runAfter.startedAt() if non-null otherwise fall back to runAfter.createdAt()
(i.e., timestamp =
Optional.ofNullable(runAfter.startedAt()).orElse(runAfter.createdAt())),
ensuring the event timestamp passed into the WorkflowQueuedEvent constructor is
never null.
- Around line 81-90: getStepStatus may call step.conclusion() when it is null,
causing an NPE; modify getStepStatus (which takes JobStep step and returns
StepStatus) to check for a null conclusion before the switch — e.g., after
handling step==null and the queued/in_progress checks, retrieve String
conclusion = step.conclusion(); if (conclusion == null) return
StepStatus.INITIAL; then switch on conclusion to map "success" -> SUCCEEDED,
"failure" -> FAILED, default -> INITIAL. Ensure you reference step.status() and
step.conclusion() in the updated logic.
In @src/main/java/com/example/github_workflow_tool/diffing/StepStatus.java:
- Around line 3-18: The enum StepStatus defines FAILED and SUCCEEDED with the
same order value (2) which is intentional for terminal states; update the source
by adding a brief comment or Javadoc near the enum constants (or above FAILED
and SUCCEEDED) explaining that FAILED and SUCCEEDED deliberately share order=2
to indicate equivalent terminal ordering, leaving the getOrder field and
constructor unchanged so behavior is preserved.
In @src/main/java/com/example/github_workflow_tool/domain/events/Event.java:
- Around line 17-22: The Event constructor lacks null validation for reference
parameters which leads to NPEs; update the Event(Instant timestamp, String
branchName, String commitSha, long runId) constructor to validate non-null for
timestamp, branchName, and commitSha (e.g., using Objects.requireNonNull or
explicit checks) and throw a clear exception
(IllegalArgumentException/NullPointerException) with a descriptive message so
downstream formatting methods cannot receive nulls.
- Line 15: Event currently creates a new CLIPrinter per instance; change this to
use a shared CLIPrinter by replacing the instance field with a single shared
instance (either a private static final CLIPrinter PRINTER = new CLIPrinter()
and update the Event#getPrinter to return that static PRINTER, or add a
CLIPrinter parameter to Event's constructor and assign the passed shared
instance to the existing field for DI); update any usages of new Event()
accordingly to pass the shared printer if using constructor injection and remove
per-instance creation so CLIPrinter is not created for every Event.
- Around line 72-84: getEventPrefix() calls abstract methods getColor() and
getEventTag() but doesn't validate their return values; update the code to
either document the non-null contract in the Javadoc of getColor() and
getEventTag() or add defensive checks inside getEventPrefix(): verify getColor()
and getEventTag() are non-null (throw IllegalStateException or
NullPointerException with a clear message if null) or substitute safe defaults
before using them in the ANSI builder; reference the methods getEventPrefix(),
getColor(), and getEventTag() when making the change.
In @src/main/java/com/example/github_workflow_tool/domain/events/JobEvent.java:
- Around line 46-54: The getEventTagIndentation() method in class JobEvent is
missing the @Override annotation; add @Override immediately above the protected
int getEventTagIndentation() declaration to match other overridden methods
(e.g., StepEvent.getEventTagIndentation()) for consistency and clarity.
In @src/main/java/com/example/github_workflow_tool/domain/events/StepEvent.java:
- Around line 51-59: The method getEventTagIndentation() overrides a superclass
method but is missing the @Override annotation; add @Override above the
getEventTagIndentation() declaration to match getOrder() and getEventTagLength()
and enable compile-time override checks.
In
@src/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.java:
- Around line 11-20: The WorkflowQueuedEvent constructor and its inputs can be
null and later cause NPEs (e.g., workflowName used in prettyPrint()/formatName()
and inherited commitSha used in substring operations); update the
WorkflowQueuedEvent constructor to validate required parameters (use
Objects.requireNonNull or explicit checks) for workflowName and ensure the base
Event constructor also validates its parameters (timestamp, branchName,
commitSha, runId) so null values are rejected early with clear exceptions.
In @src/main/java/com/example/github_workflow_tool/domain/WorkflowRun.java:
- Line 13: The WorkflowRun class's field headSha is not annotated with
@SerializedName("head_sha"), so Gson won't map the snake_case JSON property and
headSha will be null; add the annotation above the headSha field declaration (in
class WorkflowRun) to use @SerializedName("head_sha") and recompile so Gson
correctly populates headSha and prevents the NullPointerException.
In @src/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.java:
- Around line 5-9: The WorkflowRunData record currently exposes a mutable
Map<Long, Job> via the jobs component; make a defensive copy or wrap it as
unmodifiable in the record’s compact constructor so external callers cannot
mutate internal state — update the WorkflowRunData record to assign this.jobs =
Map.copyOf(jobs) or Collections.unmodifiableMap(new HashMap<>(jobs)) (and
null-check or requireNonNull on jobs) so the stored map is immutable after
construction.
In
@src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java:
- Around line 66-68: Instantiate a single CLIPrinter before the loop and reuse
it instead of newing one each iteration; locate where CLIPrinter is created in
the loop (current code uses new CLIPrinter() inside the block guarded by
toolRanBefore) and move that instantiation outside/before the loop. Also guard
the call to prettyPrintOnSeparateLines(workflowEvents) by checking if
workflowEvents is non-empty (or that the returned string is non-blank) before
calling System.out.println, to avoid printing empty lines and to handle the case
where prettyPrintOnSeparateLines returns an empty string.
- Around line 43-51: The catch-all exception handling around Thread.sleep in the
loop swallows InterruptedException and fails to restore the thread's interrupt
status; update the exception handling in the loop that uses
lastApiCallTimestamp, MINIMUM_REQUEST_INTERVAL and Thread.sleep so that when an
InterruptedException is caught you call Thread.currentThread().interrupt() and
then break or rethrow to allow graceful shutdown, while other exceptions can
still be logged/handled normally; ensure the change references the existing
variables/methods (lastApiCallTimestamp, MINIMUM_REQUEST_INTERVAL, Thread.sleep)
and the catch (Exception e) location so the interrupt flag is restored
correctly.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (22)
build.gradle.ktsgradle/libs.versions.tomlsrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/diffing/JobStatus.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/diffing/WorkflowRunStatus.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/test/java/com/example/github_workflow_tool/json/JsonServiceTests.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-05T12:38:46.923Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/domain/Repository.java:3-3
Timestamp: 2026-01-05T12:38:46.923Z
Learning: In the SerbanUntu/github-workflow-tool repository, for Java source files under src/main/java, prefer using wildcard imports for java.util (import java.util.*;) instead of explicit imports for individual java.util classes to reduce visual clutter. Ensure this aligns with project conventions; if there are naming conflicts or readability concerns, consider switching to explicit imports for those cases.
Applied to files:
src/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/diffing/WorkflowRunStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/diffing/JobStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.java
📚 Learning: 2026-01-05T12:53:43.895Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/domain/Repository.java:15-17
Timestamp: 2026-01-05T12:53:43.895Z
Learning: Guideline: In Java source files, boolean getters should follow the isXX naming pattern instead of getIsXX. For a boolean field (e.g., private boolean valid;), name the getter isValid() rather than getIsValid(). If the field is already named isXxx, keep the generated getter as isXxx (e.g., private boolean isValid; getter should be isValid()). Refactor any getIsXxx usages to isXxx, update overrides/equals/hashCode if they rely on property names, and adjust any frameworks or serialization logic that rely on bean property names. Apply consistently across the src/main/java tree, not just the specific file.
Applied to files:
src/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/diffing/WorkflowRunStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/diffing/JobStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.java
📚 Learning: 2026-01-05T13:57:46.893Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java:21-23
Timestamp: 2026-01-05T13:57:46.893Z
Learning: In Java projects, ensure that user-facing errors do not display stack traces. Replace stack traces with concise, user-friendly error messages. Log full stack traces internally (e.g., using a logger) and present a generic message to the user. Apply this guideline to all user-facing error handling across the application.
Applied to files:
src/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/diffing/WorkflowRunStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/diffing/JobStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.java
🧬 Code graph analysis (2)
src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java (3)
src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.java (1)
CLIPrinter(15-93)src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java (1)
DiffingService(15-188)src/main/java/com/example/github_workflow_tool/domain/events/Event.java (1)
Event(9-120)
src/main/java/com/example/github_workflow_tool/domain/events/Event.java (1)
src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.java (1)
CLIPrinter(15-93)
🔇 Additional comments (18)
src/main/java/com/example/github_workflow_tool/api/JobClient.java (4)
12-15: LGTM!The new imports appropriately support parallel request processing using CompletableFuture and align with the project's preference for wildcard imports for java.util.
47-53: LGTM!URI construction properly handles
URISyntaxExceptionby converting it toAPIException, which is appropriate for the stream pipeline context.
63-63: LGTM!Using
allOf().join()appropriately waits for all parallel requests to complete before proceeding to result extraction.
46-46: Verify all callers are updated for the new signature.The method signature changed from
fetchData(long runId)tofetchData(List<Long> runIds)with return type changed fromJobResponsetoList<JobResponse>. Ensure all calling code throughout the codebase has been updated to pass aList<Long>parameter and handle theList<JobResponse>return type.src/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.java (1)
1-39: LGTM!The event class follows the established pattern for domain events in this codebase. Constructor properly delegates to superclass, and the color/tag choices (YELLOW/"FINISHED") are appropriate for representing a completed job.
src/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.java (1)
1-40: LGTM!The event class follows the established pattern for step-level domain events. Constructor properly delegates to superclass, and the color/tag choices (CYAN/"STARTED") are appropriate for representing step initiation.
build.gradle.kts (1)
31-31: No action needed. jansi version 2.4.2 is the latest stable release with no known security vulnerabilities.src/main/java/com/example/github_workflow_tool/diffing/JobStatus.java (1)
3-17: LGTM!The enum is well-structured with clear lifecycle progression values. The order values correctly model the job state machine (initial → in-progress → completed).
src/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.java (1)
7-39: LGTM!The event class correctly extends
StepEvent, uses an appropriate color (RED) for failure semantics, and follows the established event pattern in this codebase.src/main/java/com/example/github_workflow_tool/diffing/WorkflowRunStatus.java (1)
3-16: Enum appears intentionally minimal; verify completeness.Unlike
JobStatus(3 states) andStepStatus(4 states), this enum only tracks INITIAL and AFTER_QUEUEING. This is likely intentional if only queue events are currently emitted for workflow runs. If you plan to add more workflow-level events (e.g., workflow started, workflow completed), consider adding corresponding status values.src/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.java (1)
7-40: LGTM!The event class correctly extends
JobEventand follows the established event pattern with appropriate color (CYAN) for started events.src/test/java/com/example/github_workflow_tool/json/JsonServiceTests.java (1)
26-89: LGTM!The test updates correctly reflect the new
headShaandstartedAtfields in theWorkflowRunrecord. The test data appropriately covers both non-null (run_started_at: "2026-01-01T19:05:00Z") and null (run_started_at: null) cases forstartedAt.src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java (1)
26-64: Well-structured diff computation logic.The
computeDiffmethod cleanly iterates through state changes, compares at workflow, job, and step levels, and aggregates events with proper sorting. The use of status enums with ordering provides a clear state machine pattern.src/main/java/com/example/github_workflow_tool/domain/events/StepEvent.java (1)
7-26: LGTM!Clean abstract class design with well-documented fields and methods. The constructor properly delegates to the parent class.
src/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.java (1)
7-40: LGTM!Clean implementation of
StepSucceededEventthat correctly extendsStepEventand provides the appropriate color (GREEN) and tag ("SUCCEEDED") for success events.src/main/java/com/example/github_workflow_tool/domain/events/JobEvent.java (1)
5-21: LGTM!Well-structured abstract class for job-related events with proper field encapsulation and constructor delegation.
src/main/java/com/example/github_workflow_tool/api/WorkflowService.java (1)
49-54: LGTM!The refactored
queryApimethod cleanly orchestrates the workflow and job fetching with a clear data flow. Using streams to extract run IDs is idiomatic.src/main/java/com/example/github_workflow_tool/domain/WorkflowRun.java (1)
10-21: The code is correctly configured. A globalFieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORESis already applied inJsonService.java, which automatically convertscreatedAtandupdatedAtto their snake_case equivalents (created_at,updated_at). The explicit@SerializedName("run_started_at")onstartedAtis necessary only because the API field name isrun_started_atrather than thestarted_atthat would result from the automatic conversion—this is intentional and correct, not an inconsistency.Likely an incorrect or invalid review comment.
- Also adds debug output capabilities to see requests made by the client.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In @src/main/java/com/example/github_workflow_tool/api/GithubClient.java:
- Line 90: The GithubClient currently instantiates a new EnvService in its
constructor (this.envService = new EnvService()), causing repeated property
loads; change the GithubClient constructor to accept an EnvService parameter and
assign it to the existing envService field instead of creating a new instance,
and remove the new EnvService() call; optionally add an overloaded constructor
or factory for backward compatibility so callers can still construct a
GithubClient without manually passing EnvService while your application code
creates a single shared EnvService and passes it to all GithubClient instances.
In @src/main/java/com/example/github_workflow_tool/api/JobClient.java:
- Around line 71-86: The stream maps HttpResponse objects and calls
jsonService.parseJobResponse(response.body()) without guarding against a null
response.body(), which can cause an NPE; update the mapping in the method that
processes futures to extract the body into a local variable, skip/filter out
null bodies (or map null to Optional.empty()) before calling
jsonService.parseJobResponse, or modify jsonService.parseJobResponse to accept
and handle null safely—refer to the lambda that processes futures, the use of
response.body(), and jsonService.parseJobResponse(response.body()) and ensure
null-checking/filtering is applied so only non-null bodies are passed to
parseJobResponse.
In @src/main/java/com/example/github_workflow_tool/api/WorkflowClient.java:
- Around line 43-45: Remove the redundant toString() call in the debug print: in
the block guarded by envService.isDebugPrintingEnabled(), replace the
System.out.println invocation that uses this.request.uri().toString() with one
that passes this.request.uri() directly (e.g., System.out.println("GET request:
" + this.request.uri())); keep the same surrounding code and logging condition
(envService.isDebugPrintingEnabled()) and the same message text.
In @src/main/java/com/example/github_workflow_tool/api/WorkflowService.java:
- Around line 23-40: mapJobsToWorkflows can NPE when a Job references a runId
not in the initial runs list; update mapJobsToWorkflows to defensively handle
missing workflow entries by checking result.get(job.runId()) before calling
.jobs().put(): if null, either skip the job and log a warning (using your
logger) or create a new WorkflowRunData placeholder (new WorkflowRunData(null,
new HashMap<>())) and insert it into result, then put the job; reference
symbols: mapJobsToWorkflows, WorkflowRunData, result, job.runId(),
response.jobs().
In @src/main/java/com/example/github_workflow_tool/cli/EnvService.java:
- Around line 12-18: The EnvService constructor currently calls
getResourceAsStream("application.properties") and passes it directly to
properties.load, which will NPE if the resource is missing; change the
constructor to first assign the InputStream from
EnvService.class.getClassLoader().getResourceAsStream("application.properties")
to a local variable, check if that variable is null, and if so throw an
EnvException with a clear message that the resource was not found (include
"application.properties" in the message); otherwise call properties.load(stream)
inside the try and close the stream in a finally or use try-with-resources to
avoid leaks.
In @src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:
- Around line 81-90: The getStepStatus method can NPE when step.conclusion() is
null; update getStepStatus to guard against a null conclusion by checking
step.conclusion() == null (or using Optional/Objects) before the switch and
return StepStatus.INITIAL for null, or use a null-safe conditional (e.g., assign
conclusion to a local String and handle null) so the switch only runs on
non-null values; refer to the getStepStatus method and the
JobStep.status()/conclusion() accessors when making the change.
- Around line 92-110: compareWorkflowRuns currently passes runAfter.startedAt()
into the WorkflowQueuedEvent but startedAt can be null for "waiting/requested"
runs; change compareWorkflowRuns to use runAfter.startedAt() if non-null
otherwise fallback to runAfter.createdAt() (or another non-null timestamp) when
constructing the WorkflowQueuedEvent so the event always has a non-null
timestamp for sorting/display; update the construction in the block that creates
WorkflowQueuedEvent to use this fallback logic.
- Around line 146-193: In compareSteps, creating
StepStartedEvent/StepFailedEvent/StepSucceededEvent uses stepAfter.startedAt()
and stepAfter.completedAt() without guarding for null which leads to NPE when
Event.compareTo() sorts events; update compareSteps to check the timestamp
values before constructing events (e.g., only add StepStartedEvent if
stepAfter.startedAt() != null, and only add failed/succeeded events if
stepAfter.completedAt() != null) or provide a safe fallback timestamp, and keep
use of StepStartedEvent, StepFailedEvent, StepSucceededEvent and
Event.compareTo() in mind so null timestamps are never added to the events list.
In @src/main/java/com/example/github_workflow_tool/domain/ToolState.java:
- Around line 5-6: ToolState currently exposes mutable Map and Set components
(runs and ignoredRunIds); add a compact constructor for the record
ToolState(Map<Long, WorkflowRunData> runs, Set<Long> ignoredRunIds) that
defensively copies the inputs (e.g., new HashMap<>(runs) / new
HashSet<>(ignoredRunIds)), wraps them with
Collections.unmodifiableMap/unmodifiableSet, and assigns those wrapped instances
back to the record components; also handle null by defaulting to
Collections.emptyMap()/emptySet() to preserve immutability and avoid NPEs.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/api/GithubClient.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowClient.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/resources/application.properties
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-05T12:38:46.923Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/domain/Repository.java:3-3
Timestamp: 2026-01-05T12:38:46.923Z
Learning: In the SerbanUntu/github-workflow-tool repository, for Java source files under src/main/java, prefer using wildcard imports for java.util (import java.util.*;) instead of explicit imports for individual java.util classes to reduce visual clutter. Ensure this aligns with project conventions; if there are naming conflicts or readability concerns, consider switching to explicit imports for those cases.
Applied to files:
src/main/java/com/example/github_workflow_tool/api/WorkflowClient.javasrc/main/java/com/example/github_workflow_tool/api/GithubClient.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.java
📚 Learning: 2026-01-05T12:53:43.895Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/domain/Repository.java:15-17
Timestamp: 2026-01-05T12:53:43.895Z
Learning: Guideline: In Java source files, boolean getters should follow the isXX naming pattern instead of getIsXX. For a boolean field (e.g., private boolean valid;), name the getter isValid() rather than getIsValid(). If the field is already named isXxx, keep the generated getter as isXxx (e.g., private boolean isValid; getter should be isValid()). Refactor any getIsXxx usages to isXxx, update overrides/equals/hashCode if they rely on property names, and adjust any frameworks or serialization logic that rely on bean property names. Apply consistently across the src/main/java tree, not just the specific file.
Applied to files:
src/main/java/com/example/github_workflow_tool/api/WorkflowClient.javasrc/main/java/com/example/github_workflow_tool/api/GithubClient.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.java
📚 Learning: 2026-01-05T13:57:46.893Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java:21-23
Timestamp: 2026-01-05T13:57:46.893Z
Learning: In Java projects, ensure that user-facing errors do not display stack traces. Replace stack traces with concise, user-friendly error messages. Log full stack traces internally (e.g., using a logger) and present a generic message to the user. Apply this guideline to all user-facing error handling across the application.
Applied to files:
src/main/java/com/example/github_workflow_tool/api/WorkflowClient.javasrc/main/java/com/example/github_workflow_tool/api/GithubClient.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.java
🧬 Code graph analysis (3)
src/main/java/com/example/github_workflow_tool/api/GithubClient.java (1)
src/main/java/com/example/github_workflow_tool/cli/EnvService.java (1)
EnvService(8-23)
src/main/java/com/example/github_workflow_tool/cli/EnvService.java (1)
src/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.java (1)
EnvException(3-7)
src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java (7)
src/main/java/com/example/github_workflow_tool/api/exceptions/APIException.java (1)
APIException(7-12)src/main/java/com/example/github_workflow_tool/cli/ArgumentParser.java (1)
ArgumentParser(14-41)src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.java (1)
CLIPrinter(15-93)src/main/java/com/example/github_workflow_tool/cli/exceptions/CLIException.java (1)
CLIException(6-13)src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java (1)
DiffingService(15-194)src/main/java/com/example/github_workflow_tool/domain/Repository.java (1)
Repository(10-74)src/main/java/com/example/github_workflow_tool/domain/events/Event.java (1)
Event(9-120)
🔇 Additional comments (11)
src/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.java (1)
3-6: LGTM! User-friendly exception class.The exception class correctly provides a user-friendly error message prefix while extending RuntimeException, aligning with the project's guideline to avoid exposing raw stack traces to users.
src/main/resources/application.properties (1)
2-2: LGTM! Sensible default for debug configuration.The debug property is correctly set to false by default, providing opt-in debug logging.
src/main/java/com/example/github_workflow_tool/cli/EnvService.java (1)
20-22: LGTM!The null-safe comparison using
Objects.equals()correctly handles the case where the "debug" property is not set.src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java (2)
26-64: LGTM!The diff computation logic correctly handles missing before-state runs and aggregates events from all levels (workflow, job, step) before sorting by timestamp.
112-144: Verify that job timestamps cannot be null when events are created.The code uses
jobAfter.startedAt()(line 122) andjobAfter.completedAt()(line 134) directly when creating events, but these fields are declared asInstantwith no nullability annotations. Gson will deserialize null JSON values to Java null, and the customInstantDeserializerdoes not handle null gracefully. While the status checks provide some context (IN_PROGRESS for started events, COMPLETED for finished events), there's no guarantee that the GitHub API provides timestamps for these status values. Add explicit null checks or document why null values are impossible.src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java (3)
43-52: LGTM!The
CLIPrinteris now correctly instantiated once before the loop, addressing the previous review feedback about unnecessary object creation on each iteration.
69-92: Verify storingnewStateinstead ofaugmentedNewStateis intentional.The diff is computed using
augmentedNewState(line 85), but the stored state is updated withnewState(line 92). Confirm this is correct—if runs that were in the old state but not innewStateshould persist in the stored state for the next iteration, consider storingaugmentedNewStateinstead.
94-97: LGTM!Error handling correctly prints user-friendly messages without stack traces, consistent with project guidelines.
Based on learnings, user-facing errors should not display stack traces.
src/main/java/com/example/github_workflow_tool/api/JobClient.java (1)
46-54: LGTM!The early return for empty input and URI construction error handling are both correct.
src/main/java/com/example/github_workflow_tool/api/WorkflowService.java (2)
50-59: LGTM!The method correctly filters out ignored runs before fetching job data, reducing unnecessary API calls.
81-92: Verify the logic ingetRunsSetDifferencefilter condition.Line 90 filters runs that are in
first, not insecond, AND present inignoredRunIds. Verify this is correct—typically, a "set difference" returns elements in the first set but not in the second. The additionalignoredRunIds.contains(run.id())constraint suggests these are specifically completed runs that should be fetched one final time. Confirm this matches the intended semantics.
- Also make sure to save the workflow data after the initial query.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In @src/main/java/com/example/github_workflow_tool/cli/EnvService.java:
- Around line 12-18: The EnvService constructor currently calls
getResourceAsStream("application.properties") and passes it directly to
properties.load(), which will cause a NullPointerException if the resource is
missing; change the constructor to obtain the InputStream into a local variable
(e.g., var in = EnvService.class.getClassLoader().getResourceAsStream(...)),
check for null and if null throw an EnvException with a clear message like
"application.properties not found on classpath", otherwise load the properties
from the stream (use try-with-resources to ensure the stream is closed) and wrap
IOExceptions in EnvException as before.
In
@src/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.java:
- Around line 5-6: Update the STORAGE description string in the StorageException
class so it explicitly states these paths are under the user's home directory;
change the LOCATION_STRING message to mention "under the user's home directory"
and give example prefixes (e.g., "~/", "$HOME/") and keep the OS-specific
examples (Windows -> %APPDATA%\\Roaming, Mac -> ~/Library/Application Support,
Unix -> ~/.config) so callers of StorageException can immediately locate the
config paths.
In @src/main/java/com/example/github_workflow_tool/cli/StorageService.java:
- Around line 67-69: The code in StorageService uses
Files.createDirectory(this.appDir) which throws NoSuchFileException if parent
directories are missing; change the call in the initialization/path-creation
logic that references this.appDir to Files.createDirectories(this.appDir) so all
necessary parent directories are created (update the block that currently checks
Files.exists(this.appDir) and then calls createDirectory to use
createDirectories instead).
- Around line 80-95: The ObjectInputStream in StorageService is created outside
the try-with-resources and may leak if an exception occurs; update the block to
include ObjectInputStream in the try-with-resources (e.g., try (FileInputStream
fileInputStream = new FileInputStream(this.filePath.toFile()); ObjectInputStream
objectInputStream = new ObjectInputStream(fileInputStream)) { ... }) remove the
explicit objectInputStream.close() call, keep reading with
objectInputStream.readObject(), then proceed to tryCastingInput and existing
error handling; this ensures streams are always closed even on exceptions.
- Around line 99-111: The save block in StorageService creates an
ObjectOutputStream but doesn't include it in the try-with-resources (only
FileOutputStream is managed), so change the try to declare both resources (e.g.,
try (FileOutputStream fileOutputStream = new
FileOutputStream(this.filePath.toFile()); ObjectOutputStream objectOutputStream
= new ObjectOutputStream(fileOutputStream)) {
objectOutputStream.writeObject(toolStateByRepo); objectOutputStream.flush(); ...
} to ensure ObjectOutputStream is closed reliably (mirroring retrieve()),
keeping existing logging via envService.isDebugPrintingEnabled() and preserving
the CannotSaveDataException behavior on IOException.
In @src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:
- Around line 183-191: The code constructs a StepSucceededEvent using
stepAfter.completedAt() which can be null and causes a NullPointerException
during event sorting; before creating the event in the block that calls
events.add(new StepSucceededEvent(...)), guard the use of
stepAfter.completedAt() by computing a non-null timestamp (e.g., prefer
stepAfter.completedAt(), fallback to stepAfter.startedAt(), then to
run.updatedAt() or another safe default) and pass that timestamp to the
StepSucceededEvent constructor, or skip adding the event if no safe timestamp is
available; update the code paths around StepSucceededEvent and events.add(...)
to use this guarded timestamp.
- Around line 157-165: StepStartedEvent is being created with
stepAfter.startedAt() which can be null and later cause a NullPointerException
during event sorting; modify the code that constructs and adds the
StepStartedEvent (the events.add(new StepStartedEvent(...)) block) to first
guard against a null startedAt() — either skip adding the StepStartedEvent when
stepAfter.startedAt() == null or supply a safe fallback timestamp (e.g., a
validated run-level timestamp or Optional-wrapped value) before constructing the
event so no null is passed into the StepStartedEvent constructor.
- Around line 170-178: StepFailedEvent is being constructed with
stepAfter.completedAt() which can be null and will cause an NPE during later
event sorting; update the construction in DiffingService so you pass a non-null
timestamp by checking stepAfter.completedAt() and falling back to a sensible
value (e.g., stepAfter.startedAt(), run.updatedAt(), or Instant.now()) before
creating the new StepFailedEvent, ensuring the chosen fallback is used
consistently for all event types that may have null completedAt().
- Around line 101-109: DiffingService currently constructs a WorkflowQueuedEvent
using runAfter.startedAt() which can be null and later breaks events.sort; guard
the creation by checking runAfter.startedAt() != null before adding the event
(or supply a non-null fallback timestamp such as runAfter.createdAt() if that
field exists), so you never pass a null timestamp into WorkflowQueuedEvent;
ensure you update the block that adds the WorkflowQueuedEvent to either skip
when startedAt is null or use the fallback, avoiding changes to Event.compareTo
here.
- Around line 123-130: The code creates a JobStartedEvent using
jobAfter.startedAt() without guarding against null, which can cause a
NullPointerException when Event.compareTo() calls this.timestamp.compareTo(...);
fix by checking jobAfter.startedAt() before constructing/adding the event (e.g.,
only create/add JobStartedEvent when jobAfter.startedAt() != null) so events
list never gets an event with a null timestamp.
In
@src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java:
- Line 27: Replace the hardcoded MINIMUM_REQUEST_INTERVAL in
GithubWorkflowToolApplication.java with a configurable value: add a property
like polling.interval.millis to application.properties (default 5000), expose it
via EnvService (e.g., getPollingIntervalMillis or a similar accessor) and use
that returned long to construct the Duration where MINIMUM_REQUEST_INTERVAL is
currently used (or replace the static constant with a method that builds
Duration.fromMillis(EnvService.getPollingIntervalMillis())). Ensure EnvService
parses the property with a sensible default and update any code that referenced
the constant to call the EnvService accessor instead.
- Line 95: The code currently calls storageService.save(toolStates) every loop
cycle; change this to persist conditionally to reduce I/O by either (a) adding a
dirty flag on the toolStates object (set it true whenever state-mutating methods
run) and only calling storageService.save(toolStates) when dirty is true (then
reset dirty), or (b) introduce an iteration counter and only call
storageService.save(toolStates) every N iterations (e.g., every 12 cycles for
1-minute persistence) or when the state actually changed by comparing a
lightweight checksum/hash of toolStates. Locate references to
storageService.save and where toolStates is mutated to set/reset the dirty flag
or update the checksum/counter accordingly.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (15)
src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/domain/Job.javasrc/main/java/com/example/github_workflow_tool/domain/JobStep.javasrc/main/java/com/example/github_workflow_tool/domain/Repository.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/resources/application.properties
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-01-05T13:57:46.893Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java:21-23
Timestamp: 2026-01-05T13:57:46.893Z
Learning: In the github-workflow-tool Java application, user-facing errors should display clean error messages without stack traces for better user experience.
Applied to files:
src/main/resources/application.properties
📚 Learning: 2026-01-05T12:38:46.923Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/domain/Repository.java:3-3
Timestamp: 2026-01-05T12:38:46.923Z
Learning: In the SerbanUntu/github-workflow-tool repository, for Java source files under src/main/java, prefer using wildcard imports for java.util (import java.util.*;) instead of explicit imports for individual java.util classes to reduce visual clutter. Ensure this aligns with project conventions; if there are naming conflicts or readability concerns, consider switching to explicit imports for those cases.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.javasrc/main/java/com/example/github_workflow_tool/domain/JobStep.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/domain/Job.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/domain/Repository.java
📚 Learning: 2026-01-05T12:53:43.895Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/domain/Repository.java:15-17
Timestamp: 2026-01-05T12:53:43.895Z
Learning: Guideline: In Java source files, boolean getters should follow the isXX naming pattern instead of getIsXX. For a boolean field (e.g., private boolean valid;), name the getter isValid() rather than getIsValid(). If the field is already named isXxx, keep the generated getter as isXxx (e.g., private boolean isValid; getter should be isValid()). Refactor any getIsXxx usages to isXxx, update overrides/equals/hashCode if they rely on property names, and adjust any frameworks or serialization logic that rely on bean property names. Apply consistently across the src/main/java tree, not just the specific file.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.javasrc/main/java/com/example/github_workflow_tool/domain/JobStep.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/domain/Job.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/domain/Repository.java
📚 Learning: 2026-01-05T13:57:46.893Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java:21-23
Timestamp: 2026-01-05T13:57:46.893Z
Learning: In Java projects, ensure that user-facing errors do not display stack traces. Replace stack traces with concise, user-friendly error messages. Log full stack traces internally (e.g., using a logger) and present a generic message to the user. Apply this guideline to all user-facing error handling across the application.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.javasrc/main/java/com/example/github_workflow_tool/domain/JobStep.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/domain/Job.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/domain/Repository.java
🧬 Code graph analysis (2)
src/main/java/com/example/github_workflow_tool/cli/StorageService.java (5)
src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.java (1)
CannotCreateStorageFileException(3-8)src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.java (1)
CannotDeleteStorageFileException(3-8)src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.java (1)
CannotSaveDataException(3-8)src/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.java (1)
StorageException(3-12)src/main/java/com/example/github_workflow_tool/domain/Repository.java (1)
Repository(11-75)
src/main/java/com/example/github_workflow_tool/cli/EnvService.java (1)
src/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.java (1)
EnvException(3-7)
🔇 Additional comments (14)
src/main/java/com/example/github_workflow_tool/domain/JobStep.java (1)
3-3: LGTM! Serialization support added correctly.The
Serializableimplementation is appropriate for the persistence feature. All component types (String,Instant,int) are inherently serializable.Also applies to: 16-16
src/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.java (1)
3-3: LGTM! Exception design is appropriate.Using an abstract
RuntimeExceptionbase class for storage-related errors is a good design. The constructor properly delegates to the superclass.Also applies to: 8-10
src/main/java/com/example/github_workflow_tool/domain/Repository.java (1)
5-5: LGTM! Serialization support enables Map key persistence.Adding
Serializableis correct for persistence. The existingequals()andhashCode()implementations ensureRepositoryworks properly as a Map key inStorageService.Also applies to: 11-11
src/main/java/com/example/github_workflow_tool/domain/ToolState.java (1)
6-6: Mutable collections compromise record immutability.The record exposes mutable
MapandSetcomponents, allowing external modification after construction. This violates immutability guarantees expected from records and can lead to subtle bugs.As previously noted, add a compact constructor to create defensive unmodifiable copies of the collections.
Likely an incorrect or invalid review comment.
src/main/resources/application.properties (1)
1-2: LGTM! Properties are correctly consumed.EnvService properly loads application.properties and reads both keys:
getAppName()retrieves thenameproperty, andisDebugPrintingEnabled()retrieves thedebugproperty. Spring's standardspring.application.nameis not referenced anywhere in the codebase, so there are no framework expectations to break.src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.java (1)
3-8: LGTM!The exception class follows best practices with a clear, user-friendly error message that guides the user to check permissions. The message appropriately references
LOCATION_STRINGfrom the parent class to provide context about where the storage file is located.src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.java (1)
3-8: LGTM!The exception class is well-structured with a clear, actionable error message. It follows the same pattern as the other storage exceptions and provides helpful guidance to users about checking permissions.
src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.java (1)
3-8: LGTM!The exception class maintains consistency with the other storage exceptions in this PR. The error message clearly communicates the issue and provides actionable guidance about checking permissions.
src/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.java (1)
6-10: The mutable Map exposure issue flagged in the previous review remains unaddressed.As noted in the earlier review, the record exposes a mutable
Map<Long, Job>that allows external callers to modify the internal state. The suggested fix using a compact constructor withMap.copyOf(jobs)would address this concern.Likely an incorrect or invalid review comment.
src/main/java/com/example/github_workflow_tool/domain/Job.java (1)
3-3: Serialization requirement satisfied: JobStep implements Serializable.The Job record's Serializable implementation is correct. JobStep is already defined as a record that implements Serializable with all serializable field types (String, int, Instant), ensuring the entire object graph serializes without issues.
src/main/java/com/example/github_workflow_tool/domain/WorkflowRun.java (1)
11-22: LGTM! Serialization and field mapping are correctly configured.The additions integrate well:
headShafield will be mapped correctly via the configuredFieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORESstartedAtcorrectly uses@SerializedName("run_started_at")for the non-standard field nameSerializableinterface enables persistence viaStorageServicesrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java (1)
54-96: LGTM! Polling loop is correctly structured.The implementation properly handles:
- Interrupt-based termination (lines 54, 65-67)
- Rate limiting between API calls (lines 56-68)
- Initial state bootstrapping (lines 73-78)
- State persistence after each cycle (line 95)
- Empty event filtering (line 88)
The continuous polling with persisted state successfully addresses the PR objectives for streaming events across executions.
src/main/java/com/example/github_workflow_tool/cli/StorageService.java (1)
23-40: LGTM! Cross-platform directory resolution is well-implemented.The OS detection and path resolution logic correctly handles Windows (AppData/Roaming), macOS (Library/Application Support), and Unix/Linux (XDG_CONFIG_HOME or .config) standards with appropriate environment variable fallbacks.
src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java (1)
135-142: Add null validation forjobAfter.completedAt()or document the API guarantee.The code assumes
completedAtis non-null whenstatusAfter == COMPLETED, but theJobrecord allows null values and there is no validation. While GitHub's API may guarantee non-nullcompleted_atfor completed jobs, this dependency should be made explicit either through defensive null checking or a documented contract with a reference to the API specification.
- Also implement many suggestions from the AI reviewer to make library methods more defensive, and to make the code more maintainable
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/main/java/com/example/github_workflow_tool/api/WorkflowService.java:
- Around line 81-92: The getRunsSetDifference method currently filters runs with
"!second.containsKey(run.id()) && ignoredRunIds.contains(run.id())" which
incorrectly keeps ignored runs; update the filter to exclude ignored runs
instead (i.e., require ignoredRunIds does NOT contain run.id()) so
runsNotInTheNewState in askForAdditionalRunData() represents runs that
disappeared from the new state and are not ignored; locate the method
getRunsSetDifference (parameters first, second, ignoredRunIds) and invert the
ignoredRunIds check to !ignoredRunIds.contains(run.id()) while leaving the rest
of the mapping (WorkflowRunData::run) intact.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
src/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowClient.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.java
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java:21-23
Timestamp: 2026-01-05T13:57:46.893Z
Learning: In the github-workflow-tool Java application, user-facing errors should display clean error messages without stack traces for better user experience.
📚 Learning: 2026-01-07T12:45:23.183Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.java:11-20
Timestamp: 2026-01-07T12:45:23.183Z
Learning: In the github-workflow-tool project (Java), fields in domain event classes (e.g., WorkflowQueuedEvent, JobEvent, StepEvent) and domain objects can be null per the GitHub API specification. Null validation should not be enforced on these fields as null is a valid state.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.java
📚 Learning: 2026-01-05T12:38:46.923Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/domain/Repository.java:3-3
Timestamp: 2026-01-05T12:38:46.923Z
Learning: In the SerbanUntu/github-workflow-tool repository, for Java source files under src/main/java, prefer using wildcard imports for java.util (import java.util.*;) instead of explicit imports for individual java.util classes to reduce visual clutter. Ensure this aligns with project conventions; if there are naming conflicts or readability concerns, consider switching to explicit imports for those cases.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowClient.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.java
📚 Learning: 2026-01-05T12:53:43.895Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/domain/Repository.java:15-17
Timestamp: 2026-01-05T12:53:43.895Z
Learning: Guideline: In Java source files, boolean getters should follow the isXX naming pattern instead of getIsXX. For a boolean field (e.g., private boolean valid;), name the getter isValid() rather than getIsValid(). If the field is already named isXxx, keep the generated getter as isXxx (e.g., private boolean isValid; getter should be isValid()). Refactor any getIsXxx usages to isXxx, update overrides/equals/hashCode if they rely on property names, and adjust any frameworks or serialization logic that rely on bean property names. Apply consistently across the src/main/java tree, not just the specific file.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowClient.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.java
📚 Learning: 2026-01-05T13:57:46.893Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java:21-23
Timestamp: 2026-01-05T13:57:46.893Z
Learning: In Java projects, ensure that user-facing errors do not display stack traces. Replace stack traces with concise, user-friendly error messages. Log full stack traces internally (e.g., using a logger) and present a generic message to the user. Apply this guideline to all user-facing error handling across the application.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowClient.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.java
📚 Learning: 2026-01-07T12:51:57.721Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:101-109
Timestamp: 2026-01-07T12:51:57.721Z
Learning: In DiffingService.java, when evaluating a WorkflowRun, if the status is neither 'waiting' nor 'requested' (i.e., AFTER_QUEUEING in your diffing logic), you can rely on startedAt() being non-null as per the GitHub API spec. Do not perform null checks for startedAt in this code path; document this guarantee and access startedAt() directly, and optionally add a unit test asserting non-null in that path.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowClient.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.java
📚 Learning: 2026-01-07T12:48:39.017Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:183-191
Timestamp: 2026-01-07T12:48:39.017Z
Learning: In Java files handling GitHub workflow API responses, assume that a step's completedAt field is non-null when the step's conclusion is set (e.g., 'success', 'failure'); the presence of a conclusion implies the step is completed. Update code to rely on completedAt being non-null in these cases and avoid false negatives from null checks. This guideline applies to all workflow step result handling across the project.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowClient.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.java
📚 Learning: 2026-01-07T12:49:09.426Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:170-178
Timestamp: 2026-01-07T12:49:09.426Z
Learning: In any Java code within the project, when modeling GitHub workflow steps, if a JobStep has a non-null conclusion (e.g., 'failure' or 'success'), the corresponding completedAt field is guaranteed to be non-null according to the GitHub API specification. Ensure your parsing and JSON mapping respect this invariant for all completed steps with a conclusion, and add nullability checks and appropriate null-safe handling in DiffingService and related types.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/api/JobClient.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowClient.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.java
📚 Learning: 2026-01-07T12:48:52.865Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:157-165
Timestamp: 2026-01-07T12:48:52.865Z
Learning: In the github-workflow-tool project (Java), when a JobStep has status "in_progress", the startedAt() field is guaranteed to be non-null per the GitHub API specification. Similarly, when a Job has status "in_progress", the startedAt() field is guaranteed to be non-null.
Applied to files:
src/main/java/com/example/github_workflow_tool/diffing/StepStatus.java
📚 Learning: 2026-01-07T12:45:23.183Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.java:11-20
Timestamp: 2026-01-07T12:45:23.183Z
Learning: Do not enforce non-null validation for fields in domain event classes and domain objects in the github-workflow-tool project. According to the GitHub API specification, fields like those in WorkflowQueuedEvent, JobEvent, and StepEvent can be null and represent valid states. Update code reviews to allow null values where the API permits, avoid extra null checks, and adjust tests and validation rules to reflect nullable fields in the domain/events package.
Applied to files:
src/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.java
🧬 Code graph analysis (3)
src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.java (1)
src/main/java/com/example/github_workflow_tool/domain/events/Event.java (1)
Event(9-120)
src/main/java/com/example/github_workflow_tool/cli/EnvService.java (1)
src/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.java (1)
EnvException(3-7)
src/main/java/com/example/github_workflow_tool/cli/StorageService.java (5)
src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.java (1)
CannotCreateStorageFileException(3-8)src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.java (1)
CannotDeleteStorageFileException(3-8)src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.java (1)
CannotSaveDataException(3-8)src/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.java (1)
StorageException(3-12)src/main/java/com/example/github_workflow_tool/domain/Repository.java (1)
Repository(11-75)
🔇 Additional comments (21)
src/main/java/com/example/github_workflow_tool/diffing/StepStatus.java (1)
3-18: Well-documented enum design.The comment on line 7 effectively addresses the previous review feedback by documenting the intentional shared order value for terminal states. The enum structure cleanly models step progression with appropriate ordering semantics.
src/main/java/com/example/github_workflow_tool/api/WorkflowClient.java (1)
43-45: LGTM! Debug logging implementation is clean.The debug printing logic is well-implemented with proper conditional checking and clear message formatting.
src/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.java (1)
1-12: LGTM! StorageException base class is well-structured.The exception class appropriately extends RuntimeException and provides clear platform-specific path guidance with proper prefixes.
src/main/java/com/example/github_workflow_tool/cli/EnvService.java (1)
26-28: LGTM! Null-safe property comparison.Using
Objects.equalsprovides proper null-safe comparison for the debug property check.src/main/java/com/example/github_workflow_tool/domain/events/JobEvent.java (1)
1-71: LGTM! JobEvent base class is well-structured.The abstract class properly extends Event, includes all necessary @OverRide annotations, and implements consistent formatting behavior for job-related events.
src/main/java/com/example/github_workflow_tool/domain/events/StepEvent.java (1)
1-76: LGTM! StepEvent base class is well-structured.The abstract class properly extends Event, includes all necessary @OverRide annotations, and implements consistent formatting behavior for step-related events with appropriate indentation.
src/main/java/com/example/github_workflow_tool/api/JobClient.java (3)
46-54: LGTM on the URI building and null/empty guard.The early return for null or empty
runIdsand the URI construction with exception handling are well-implemented.
56-69: Parallel request execution looks good.The futures are created correctly with debug logging, and
CompletableFuture.allOf(...).join()properly waits for all requests to complete before processing results.
71-88: Previous issues addressed: null body handling and cause extraction.The code now correctly:
- Extracts the cause from
ExecutionException(lines 80-81)- Filters out null responses (line 84) and null bodies (line 86) before parsing
This addresses the previously flagged NPE concerns.
src/main/java/com/example/github_workflow_tool/cli/StorageService.java (4)
23-40: OS-specific path resolution is well-implemented.The constructor correctly handles Windows, macOS, and Unix/Linux environments with appropriate fallbacks for environment variables.
42-55: Type-safe casting with runtime validation.The
tryCastingInputmethod properly validates map entries before the unchecked cast, which is the correct pattern for deserializing generic types.
80-93: Previous resource leak issue addressed.The code now correctly uses nested try-with-resources blocks for both
FileInputStreamandObjectInputStream, ensuring proper resource cleanup.
96-109: Previous ObjectOutputStream concern addressed.The
savemethod now uses nested try-with-resources for bothFileOutputStreamandObjectOutputStream, consistent with theretrievemethod.src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.java (4)
17-19: Previous concern addressed: DateTimeFormatter is now static.The formatter is correctly declared as a
private static finalfield, ensuring thread-safe reuse without per-call allocation.
30-57: Previous division-by-zero issue addressed.The code now checks
if (average == 0.0f)and provides a safe fallback (gray: 128, 128, 128) instead of dividing by zero.
59-61: Previous null validation concern addressed.The
formatTagmethod now handles null by using a ternary operator to substitute an empty string.
74-80: Previous null validation concerns addressed throughout.All formatting methods now handle null inputs defensively:
formatName: returns empty string for null/blankprettyPrintOnSeparateLines: returns empty string for null/empty liststripAnsi: returns empty string for nullgetRepeatedString: handles null pattern and negative lengthgetPaddingBeforeBranch: handles null branchNameAlso applies to: 82-87, 89-92, 94-96, 98-107
src/main/java/com/example/github_workflow_tool/api/WorkflowService.java (4)
23-40: Implementation looks correct given API guarantees.The
mapJobsToWorkflowsmethod correctly builds the mapping. Per the author's prior clarification, the API guarantees jobs will only reference runs that were fetched, so the lookup is safe.
50-59: Filtering by ignoredRuns is correctly applied.The method fetches all workflow runs but only requests job details for runs not in the ignored set, optimizing API calls.
61-70: Previous issue addressed: throws declaration added.The
askForAdditionalRunDatamethod now correctly declaresthrows APIException, CLIException.
72-79: LGTM for getRunsIdsToIgnore.The method correctly identifies completed runs (non-null conclusion) to be excluded from future API fetches.
- Also bump version to 0.1.0 from 0.1.0-SNAPSHOT - Also fix a bug regarding fetching data about runs that are in the previous state but were not returned by the global runs call.
Owner
Author
|
@coderabbitai Can you check whether the app terminates gracefully when interrupted, for example via Ctrl+C from the console? Especially with regard to HTTP connections and file stream connections. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 4684 🏁 Script executed: Length of output: 4525 🏁 Script executed: Length of output: 3838 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 2527 🏁 Script executed: Length of output: 3593 @SerbanUntu, I've analyzed the application's graceful termination behavior. Here are my findings: ✅ What's Working Well
❌ Critical Issues1. No Shutdown Hook for State PersistenceWhen Ctrl+C is pressed, the application exits without saving the current state. There's no shutdown hook registered to persist data before termination. This means:
Recommendation: Add a shutdown hook in Runtime.getRuntime().addShutdownHook(new Thread(() -> {
try {
storageService.save(toolStates);
System.out.println("\nState saved. Exiting gracefully.");
} catch (Exception e) {
System.err.println("Failed to save state on shutdown: " + e.getMessage());
}
}));2. HTTP Interruption Causes Abrupt ExitIf an interrupt occurs during an HTTP operation:
Recommendation: Check interrupt status before throwing in HTTP error handlers, or handle 3. Uninterruptible Parallel HTTP Requests
Recommendation: Replace 4. HttpClient Not Explicitly ClosedThe Recommendation: Consider making HttpClient 🎯 SummaryThe application does NOT terminate gracefully when interrupted. Specifically:
This aligns with the open TODO item: "Handle graceful termination (open)". Would you like me to help draft a complete graceful shutdown implementation? 🧠 Learnings used |
Owner
Author
|
@coderabbitai This is very insightful, but I don't want to overcomplicate my code with recovery logic. I just want the app to exit when the user terminates it. Resources are deallocated by the OS. Losing one iteration of the state will just cause the same logs to be printed one more time, which is not that big of an issue. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. @SerbanUntu, that's a reasonable trade-off for simplicity! If you're comfortable with duplicate logs on restart and relying on OS cleanup, then the current implementation is fine for your use case. One small FYI: when you press Ctrl+C during the parallel HTTP requests in Otherwise, the interrupt flag check in your main loop should work well for most scenarios. Thanks for the clarification on your design preferences! 🧠 Learnings used |
This was
linked to
issues
Jan 7, 2026
- Also fix event ordering logic
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In
@src/main/java/com/example/github_workflow_tool/api/exceptions/ServerError.java:
- Around line 12-17: The Javadoc for the method in class ServerError uses
"between 500 and 599" which can be ambiguous; update the @param description for
the status code in ServerError to read "from 500 to 599 inclusive" (or "500–599
inclusive") so it clearly matches the @throws clause that requires the value to
be within that inclusive range.
In @src/main/java/com/example/github_workflow_tool/cli/EnvService.java:
- Around line 37-43: EnvService currently returns null from getAppName() when
the "name" property is missing causing NPEs downstream (e.g.,
StorageService.path.resolve(this.envService.getAppName())). Validate the
presence of the "name" property during EnvService initialization (e.g., in the
constructor or init method) and if absent throw an EnvException with a clear
message; update getAppName() to either return the validated value or keep it
as-is but rely on the constructor check to guarantee non-null. Ensure the thrown
exception type is EnvException and reference EnvService and getAppName() so
callers (like StorageService) can assume a non-null app name.
In
@src/main/java/com/example/github_workflow_tool/cli/exceptions/GenericClientError.java:
- Around line 12-17: No change required—the Javadoc for the method in
GenericClientError accurately documents validation, params, return, and
exception; if you want to tighten it further, shorten the comment to a single
concise sentence describing the method's purpose and validation or make it
package-private/private documentation briefer, but no functional code changes
are needed.
In
@src/main/java/com/example/github_workflow_tool/cli/exceptions/TooFewArgumentsException.java:
- Around line 8-10: The TooFewArgumentsException constructor currently builds a
message from numberOfArguments; add defensive validation inside
TooFewArgumentsException(int numberOfArguments) to guard against misuse by
checking if numberOfArguments < 2 and if not either throw an
IllegalArgumentException (e.g., "TooFewArgumentsException must be constructed
with numberOfArguments < 2") or normalize the message to reflect a minimum of 2;
update the constructor logic to perform this check before calling super(...) so
misleading messages (like "Expected 2, got 3") cannot be produced.
In @src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:
- Around line 53-59: The current loop matches JobStep objects by list index
which can mis-pair steps if the workflow's step order changes; instead, build a
Map<Integer, JobStep> keyed by JobStep::number from jobBefore (handle null by
using an empty map), then iterate jobAfter.steps() and for each JobStep use
map.get(stepAfter.number()) as stepBefore and pass those to
compareSteps(runAfter, jobAfter, stepBefore, stepAfter); update the loop around
jobAfter, jobBefore and remove index-based access to ensure robust matching by
step.number.
In @src/test/java/com/example/github_workflow_tool/cli/CLIPrinterTests.java:
- Around line 24-28: The test method name formatStepNumberSingleDigitTest is
misleading because the CsvSource includes multi-digit inputs; rename the test
method to a neutral name like formatStepNumberTest (or
formatStepNumberParameterizedTest) so it accurately reflects that
sut.formatStepNumber is validated across single- and multi-digit values; update
the method declaration and any references to the old name accordingly.
In
@src/test/java/com/example/github_workflow_tool/diffing/DiffingServiceTests.java:
- Around line 27-170: The setUp() method is too long and should use fixture
builders to improve readability; extract small helper methods like makeStep(...)
to construct JobStep, makeJob(...) for Job, and makeRun(...) for WorkflowRun
(and optionally makeRunData(...) for WorkflowRunData), then replace inline
constructors in setUp() with calls to these helpers and keep the final
assignments to before and after unchanged; update references to JobStep, Job,
WorkflowRun, WorkflowRunData, setUp(), before, and after accordingly so tests
remain identical but the setup is concise and easier to maintain.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (39)
.github/workflows/main.ymlbuild.gradle.ktssrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/api/exceptions/ServerError.javasrc/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CLIException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/GenericClientError.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InexistentRepositoryException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InvalidAccessTokenException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InvalidRepositoryStringException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/TooFewArgumentsException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/TooManyArgumentsException.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/diffing/JobStatus.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/diffing/WorkflowRunStatus.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/test/java/com/example/github_workflow_tool/cli/CLIPrinterTests.javasrc/test/java/com/example/github_workflow_tool/diffing/DiffingServiceTests.javasrc/test/java/com/example/github_workflow_tool/json/JsonServiceTests.java
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2026-01-05T12:38:46.923Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/domain/Repository.java:3-3
Timestamp: 2026-01-05T12:38:46.923Z
Learning: In the SerbanUntu/github-workflow-tool repository, for Java source files under src/main/java, prefer using wildcard imports for java.util (import java.util.*;) instead of explicit imports for individual java.util classes to reduce visual clutter. Ensure this aligns with project conventions; if there are naming conflicts or readability concerns, consider switching to explicit imports for those cases.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/exceptions/CLIException.javasrc/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/diffing/WorkflowRunStatus.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/GenericClientError.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InexistentRepositoryException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/java/com/example/github_workflow_tool/api/exceptions/ServerError.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InvalidRepositoryStringException.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/TooFewArgumentsException.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InvalidAccessTokenException.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.javasrc/main/java/com/example/github_workflow_tool/diffing/JobStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/TooManyArgumentsException.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.java
📚 Learning: 2026-01-05T12:53:43.895Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/domain/Repository.java:15-17
Timestamp: 2026-01-05T12:53:43.895Z
Learning: Guideline: In Java source files, boolean getters should follow the isXX naming pattern instead of getIsXX. For a boolean field (e.g., private boolean valid;), name the getter isValid() rather than getIsValid(). If the field is already named isXxx, keep the generated getter as isXxx (e.g., private boolean isValid; getter should be isValid()). Refactor any getIsXxx usages to isXxx, update overrides/equals/hashCode if they rely on property names, and adjust any frameworks or serialization logic that rely on bean property names. Apply consistently across the src/main/java tree, not just the specific file.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/exceptions/CLIException.javasrc/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/diffing/WorkflowRunStatus.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/GenericClientError.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InexistentRepositoryException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/java/com/example/github_workflow_tool/api/exceptions/ServerError.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InvalidRepositoryStringException.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/TooFewArgumentsException.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InvalidAccessTokenException.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.javasrc/main/java/com/example/github_workflow_tool/diffing/JobStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/TooManyArgumentsException.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.java
📚 Learning: 2026-01-05T13:57:46.893Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java:21-23
Timestamp: 2026-01-05T13:57:46.893Z
Learning: In Java projects, ensure that user-facing errors do not display stack traces. Replace stack traces with concise, user-friendly error messages. Log full stack traces internally (e.g., using a logger) and present a generic message to the user. Apply this guideline to all user-facing error handling across the application.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/exceptions/CLIException.javasrc/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/diffing/WorkflowRunStatus.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/GenericClientError.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InexistentRepositoryException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/java/com/example/github_workflow_tool/api/exceptions/ServerError.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InvalidRepositoryStringException.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/TooFewArgumentsException.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InvalidAccessTokenException.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.javasrc/main/java/com/example/github_workflow_tool/diffing/JobStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/TooManyArgumentsException.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.java
📚 Learning: 2026-01-07T12:51:57.721Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:101-109
Timestamp: 2026-01-07T12:51:57.721Z
Learning: In DiffingService.java, when evaluating a WorkflowRun, if the status is neither 'waiting' nor 'requested' (i.e., AFTER_QUEUEING in your diffing logic), you can rely on startedAt() being non-null as per the GitHub API spec. Do not perform null checks for startedAt in this code path; document this guarantee and access startedAt() directly, and optionally add a unit test asserting non-null in that path.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/exceptions/CLIException.javasrc/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/diffing/WorkflowRunStatus.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/GenericClientError.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InexistentRepositoryException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/java/com/example/github_workflow_tool/api/exceptions/ServerError.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InvalidRepositoryStringException.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/TooFewArgumentsException.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InvalidAccessTokenException.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.javasrc/main/java/com/example/github_workflow_tool/diffing/JobStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/TooManyArgumentsException.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.java
📚 Learning: 2026-01-07T12:48:39.017Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:183-191
Timestamp: 2026-01-07T12:48:39.017Z
Learning: In Java files handling GitHub workflow API responses, assume that a step's completedAt field is non-null when the step's conclusion is set (e.g., 'success', 'failure'); the presence of a conclusion implies the step is completed. Update code to rely on completedAt being non-null in these cases and avoid false negatives from null checks. This guideline applies to all workflow step result handling across the project.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/exceptions/CLIException.javasrc/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/diffing/WorkflowRunStatus.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/GenericClientError.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InexistentRepositoryException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/java/com/example/github_workflow_tool/api/exceptions/ServerError.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/test/java/com/example/github_workflow_tool/diffing/DiffingServiceTests.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InvalidRepositoryStringException.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/TooFewArgumentsException.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InvalidAccessTokenException.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.javasrc/test/java/com/example/github_workflow_tool/cli/CLIPrinterTests.javasrc/main/java/com/example/github_workflow_tool/diffing/JobStatus.javasrc/test/java/com/example/github_workflow_tool/json/JsonServiceTests.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/TooManyArgumentsException.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.java
📚 Learning: 2026-01-07T12:49:09.426Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:170-178
Timestamp: 2026-01-07T12:49:09.426Z
Learning: In any Java code within the project, when modeling GitHub workflow steps, if a JobStep has a non-null conclusion (e.g., 'failure' or 'success'), the corresponding completedAt field is guaranteed to be non-null according to the GitHub API specification. Ensure your parsing and JSON mapping respect this invariant for all completed steps with a conclusion, and add nullability checks and appropriate null-safe handling in DiffingService and related types.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/exceptions/CLIException.javasrc/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/diffing/WorkflowRunStatus.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/GenericClientError.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InexistentRepositoryException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.javasrc/main/java/com/example/github_workflow_tool/api/exceptions/ServerError.javasrc/main/java/com/example/github_workflow_tool/cli/EnvService.javasrc/test/java/com/example/github_workflow_tool/diffing/DiffingServiceTests.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InvalidRepositoryStringException.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/TooFewArgumentsException.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/InvalidAccessTokenException.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/api/WorkflowService.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.javasrc/test/java/com/example/github_workflow_tool/cli/CLIPrinterTests.javasrc/main/java/com/example/github_workflow_tool/diffing/JobStatus.javasrc/test/java/com/example/github_workflow_tool/json/JsonServiceTests.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/cli/exceptions/TooManyArgumentsException.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.javasrc/main/java/com/example/github_workflow_tool/cli/StorageService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.java
📚 Learning: 2026-01-07T12:45:23.183Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.java:11-20
Timestamp: 2026-01-07T12:45:23.183Z
Learning: In the github-workflow-tool project (Java), fields in domain event classes (e.g., WorkflowQueuedEvent, JobEvent, StepEvent) and domain objects can be null per the GitHub API specification. Null validation should not be enforced on these fields as null is a valid state.
Applied to files:
src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.java
📚 Learning: 2026-01-07T12:48:52.865Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:157-165
Timestamp: 2026-01-07T12:48:52.865Z
Learning: In the github-workflow-tool project (Java), when a JobStep has status "in_progress", the startedAt() field is guaranteed to be non-null per the GitHub API specification. Similarly, when a Job has status "in_progress", the startedAt() field is guaranteed to be non-null.
Applied to files:
src/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/domain/WorkflowRun.javasrc/main/java/com/example/github_workflow_tool/domain/ToolState.javasrc/main/java/com/example/github_workflow_tool/diffing/StepStatus.javasrc/main/java/com/example/github_workflow_tool/diffing/JobStatus.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.java
📚 Learning: 2026-01-07T12:51:57.721Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:101-109
Timestamp: 2026-01-07T12:51:57.721Z
Learning: In the github-workflow-tool project (Java), when a WorkflowRun has a status other than "waiting" or "requested" (i.e., when it reaches AFTER_QUEUEING state in the diffing logic), the startedAt() field is guaranteed to be non-null per the GitHub API specification.
Applied to files:
src/test/java/com/example/github_workflow_tool/diffing/DiffingServiceTests.javasrc/test/java/com/example/github_workflow_tool/json/JsonServiceTests.java
📚 Learning: 2026-01-07T12:45:23.183Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.java:11-20
Timestamp: 2026-01-07T12:45:23.183Z
Learning: Do not enforce non-null validation for fields in domain event classes and domain objects in the github-workflow-tool project. According to the GitHub API specification, fields like those in WorkflowQueuedEvent, JobEvent, and StepEvent can be null and represent valid states. Update code reviews to allow null values where the API permits, avoid extra null checks, and adjust tests and validation rules to reflect nullable fields in the domain/events package.
Applied to files:
src/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.java
🧬 Code graph analysis (4)
src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.java (1)
src/main/java/com/example/github_workflow_tool/domain/events/Event.java (1)
Event(13-151)
src/main/java/com/example/github_workflow_tool/cli/EnvService.java (1)
src/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.java (1)
EnvException(6-11)
src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java (8)
src/main/java/com/example/github_workflow_tool/api/exceptions/APIException.java (1)
APIException(7-12)src/main/java/com/example/github_workflow_tool/cli/ArgumentParser.java (1)
ArgumentParser(14-41)src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.java (1)
CLIPrinter(15-167)src/main/java/com/example/github_workflow_tool/cli/StorageService.java (1)
StorageService(19-136)src/main/java/com/example/github_workflow_tool/cli/exceptions/CLIException.java (1)
CLIException(6-16)src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java (1)
DiffingService(15-236)src/main/java/com/example/github_workflow_tool/domain/Repository.java (1)
Repository(11-75)src/main/java/com/example/github_workflow_tool/domain/events/Event.java (1)
Event(13-151)
src/main/java/com/example/github_workflow_tool/cli/StorageService.java (5)
src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.java (1)
CannotCreateStorageFileException(6-11)src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.java (1)
CannotDeleteStorageFileException(6-11)src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.java (1)
CannotSaveDataException(6-11)src/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.java (1)
StorageException(6-18)src/main/java/com/example/github_workflow_tool/domain/Repository.java (1)
Repository(11-75)
🔇 Additional comments (39)
src/main/java/com/example/github_workflow_tool/cli/exceptions/CLIException.java (1)
8-10: Javadoc addition improves clarity.Documenting the
usageStringfield is a helpful improvement that makes the codebase more maintainable.src/main/java/com/example/github_workflow_tool/cli/exceptions/InexistentRepositoryException.java (1)
10-12: No action needed. The Repository class already overrides toString() to return the "owner/repo" format (line 72-74 of Repository.java), so the error message will display user-friendly output like "The repository you entered does not exist, or you do not have access to it: owner/repo" rather than a cryptic hash code.build.gradle.kts (2)
6-6: LGTM! Version bump aligns with release preparation.The version change from "0.1.0-SNAPSHOT" to "0.1.0" is appropriate for the release and matches the PR objectives.
31-31: LGTM! JANSI dependency supports CLI formatting.The addition of the JANSI library enables ANSI color/styling in the CLI output, which is used by the newly introduced CLIPrinter and event formatting components.
src/main/java/com/example/github_workflow_tool/domain/WorkflowRunData.java (1)
1-16: LGTM! Clean record design for workflow run data.The record appropriately encapsulates a workflow run with its associated jobs, and the Serializable implementation supports the state persistence requirements of the PR. The wildcard import follows project conventions.
src/main/java/com/example/github_workflow_tool/diffing/WorkflowRunStatus.java (1)
1-19: LGTM! Well-structured enum for workflow state tracking.The enum provides clear state progression with explicit ordering, which supports the diffing and event ordering requirements of the PR. The implementation is straightforward and correct.
.github/workflows/main.yml (4)
7-11: LGTM! Proper release trigger configuration.The workflow correctly triggers on version tags and grants necessary write permissions for creating GitHub releases. This supports the automated release objectives of the PR.
51-56: LGTM! Dynamic version extraction.Extracting the version from Gradle properties ensures consistency between the build configuration and release artifacts, avoiding hardcoded values.
58-68: LGTM! Portable distribution creation.The use of Gradle's
installDisttask creates a standalone distribution as specified in the PR objectives. The zip naming convention includes the version for clear artifact identification.
70-78: Confirm whether creating/updating a "latest" release on every main branch push is intentional.The workflow creates or updates a "latest" release on every push to the main branch due to the condition on line 36 (
github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/')). WithallowUpdates: trueandreplacesArtifacts: true, each main commit overwrites the previous "latest" release, which may or may not be the desired behavior. If this is intentional for rolling latest builds, it's fine; otherwise, consider restricting releases to tagged versions only by changing the job condition tostartsWith(github.ref, 'refs/tags/').src/main/java/com/example/github_workflow_tool/domain/WorkflowRun.java (2)
14-14: LGTM! Field additions properly configured.The
headShafield relies on the automatic naming policy conversion (headSha → head_sha), whilestartedAtcorrectly uses@SerializedName("run_started_at")because it maps to a differently-named JSON field that wouldn't be covered by the automatic conversion.Also applies to: 21-21
22-22: LGTM! Serializable implementation supports state persistence.The addition of Serializable is necessary for the state persistence functionality introduced in this PR, working in conjunction with WorkflowRunData and StorageService.
src/main/java/com/example/github_workflow_tool/cli/exceptions/EnvException.java (1)
1-11: LGTM! Well-designed exception for environment errors.The exception class follows best practices with a clear, user-friendly message that provides context without exposing stack traces.
src/main/java/com/example/github_workflow_tool/domain/ToolState.java (1)
1-13: LGTM! Awaiting immutability improvements in #15.The record structure is clear and well-documented. As noted in previous review feedback, the mutable collection components will be addressed in #15 to ensure full immutability.
src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotDeleteStorageFileException.java (1)
1-11: LGTM! Clear exception with actionable guidance.The exception message is user-friendly and provides concrete guidance for resolution. The inheritance hierarchy is appropriate.
src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotCreateStorageFileException.java (1)
1-11: LGTM! Well-designed exception for file creation failures.The exception provides a clear, actionable message and follows the established storage exception pattern.
src/main/java/com/example/github_workflow_tool/cli/exceptions/CannotSaveDataException.java (1)
1-11: LGTM! Appropriate exception for data persistence failures.The exception follows the established pattern and provides clear guidance. The storage exception hierarchy now covers the key failure scenarios (create, delete, save).
src/main/java/com/example/github_workflow_tool/diffing/JobStatus.java (1)
1-20: LGTM!Clean enum implementation with ordering support. The structure aligns well with the diffing service requirements.
src/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.java (1)
1-55: LGTM!The event class is well-structured with clear javadoc and appropriate color coding (CYAN for started events).
src/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.java (1)
1-43: LGTM!The event class correctly uses Color.RED for failed steps. Note that unlike StepStartedEvent, this class doesn't override
getOrder(), so it will use the default ordering from StepEvent.src/main/java/com/example/github_workflow_tool/diffing/StepStatus.java (1)
1-21: LGTM!The enum is well-structured with clear ordering semantics. The inline comment on line 10 appropriately documents that
FAILEDandSUCCEEDEDshare the same order value since they represent equivalent terminal points in time.src/test/java/com/example/github_workflow_tool/json/JsonServiceTests.java (2)
17-26: Good use of a shared reference date for deterministic tests.The
referenceDateconstant andmakeInstanthelper provide clean, readable timestamp construction for test fixtures.
28-91: LGTM!The test properly covers both scenarios: a workflow run with a non-null
run_started_at(first run) and one with a nullrun_started_at(second run, line 57/86). This aligns with the API behavior where runs in "waiting" or "requested" status may have nullstartedAt.src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java (2)
97-108: Good fix for the NPE mentioned in PR objectives.The null check for
step.conclusion()at line 101 correctly handles the case where a step's status is neither "queued" nor "in_progress" but the conclusion hasn't been set yet. This addresses the NullPointerException issue mentioned in the PR TODOs.
27-65: LGTM!The
computeDiffmethod is well-structured with clear iteration over workflow runs, jobs, and steps. The final sort on line 63 ensures events are returned in chronological order using theEvent.compareToimplementation.src/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.java (1)
1-55: LGTM!Clean event class implementation with appropriate ordering (2), color (CYAN), and tag ("STARTED") for job start events. The Javadoc clearly explains the
getOrder()semantics.src/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.java (1)
1-54: LGTM!Consistent event class implementation. The higher order value (5) correctly ensures job finished events sort after started events when timestamps are equal.
src/main/java/com/example/github_workflow_tool/cli/exceptions/StorageException.java (1)
1-18: LGTM!The
LOCATION_STRINGnow uses clear platform-specific prefixes (%APPDATA%,~/) making it immediately obvious to users where to find the application data. This addresses the previous review feedback about clarifying storage paths.src/test/java/com/example/github_workflow_tool/diffing/DiffingServiceTests.java (2)
172-270: Good test coverage with helpful inline documentation.The expected event list with inline comments mapping each field to its source (e.g.,
// step2After.completedAt()) is excellent for maintainability. The test verifies the exact event sequence including proper chronological ordering.
272-275: LGTM!Good edge case test verifying that diffing identical states produces an empty event list.
src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java (1)
34-101: LGTM! Well-structured polling implementation.The main method correctly orchestrates the polling loop with proper interrupt handling, state persistence, and user-friendly error messages. Key improvements from previous reviews have been successfully addressed:
- InterruptedException is properly caught, the interrupt flag is restored, and the loop exits cleanly (lines 65-67)
- CLIPrinter is instantiated once outside the loop (line 44)
- Empty event lists are filtered before printing (line 89)
- User-facing exceptions display clean messages without stack traces (lines 98-101)
The architectural decisions around state persistence frequency and graceful shutdown align with the project's scope and the maintainer's accepted trade-offs.
src/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.java (1)
10-42: LGTM! Clean event implementation.The class correctly extends
StepEventand implements the required abstract methods. The GREEN color and "SUCCEEDED" tag appropriately represent a successful step completion. The constructor properly delegates all parameters to the superclass.src/main/java/com/example/github_workflow_tool/domain/events/StepEvent.java (1)
9-92: LGTM! Well-designed abstract base class.The abstract
StepEventclass provides a solid foundation for step lifecycle events. The implementation correctly:
- Extends Event with step-specific fields (stepName, stepNumber, jobName)
- Overrides all required abstract methods with
@Overrideannotations (lines 37, 48, 59)- Implements proper
equals()andhashCode()with delegation to superclass (lines 81-92)- Formats output using the inherited printer for consistency (lines 70-79)
src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.java (1)
15-167: Excellent improvements to robustness and efficiency!The CLIPrinter class now demonstrates comprehensive defensive programming. All previously identified issues have been successfully addressed:
DateTimeFormatteris now a static final field for efficiency (lines 18-19)- Division by zero is prevented in
formatRunId()with an explicit check (lines 48-53)- All formatting methods handle null/invalid inputs gracefully:
formatTag()handles null tags (line 78)formatCommitSha()returns question marks for null/blank values (lines 99-100)formatName()returns empty string for null/blank names (line 111)prettyPrintOnSeparateLines()handles null/empty lists (line 124)stripAnsi()returns empty string for null input (line 136)getRepeatedString()handles null patterns and negative lengths (line 147)getPaddingBeforeBranch()handles null branchName (line 159)This comprehensive null handling should help prevent the NullPointerException mentioned in the PR objectives.
src/main/java/com/example/github_workflow_tool/domain/events/JobEvent.java (1)
9-76: LGTM! Consistent abstract base class implementation.The abstract
JobEventclass mirrors the design ofStepEventand provides appropriate structure for job lifecycle events. The implementation correctly:
- Extends Event with job-specific fields (jobName, workflowName)
- Includes
@Overrideannotations on all overridden methods (lines 33, 44)- Implements proper
equals()andhashCode()with superclass delegation (lines 64-75)- Formats output consistently using the printer's
formatName()method (lines 55-62)src/main/java/com/example/github_workflow_tool/cli/StorageService.java (2)
100-109: Excellent fix: ObjectInputStream now properly managed in try-with-resources.The nested try-with-resources pattern correctly addresses the critical resource leak issue flagged in previous reviews. Both FileInputStream and ObjectInputStream are now guaranteed to be closed, even if exceptions occur during deserialization.
123-131: Good: Consistent resource management in save() method.The nested try-with-resources pattern for ObjectOutputStream matches the retrieve() method implementation and ensures explicit, reliable cleanup. This addresses the consistency concern raised in previous reviews.
src/main/java/com/example/github_workflow_tool/api/WorkflowService.java (2)
79-88: Verified: Method correctly declares thrown exceptions.The
askForAdditionalRunDatamethod signature now properly declaresthrows APIException, CLIException, addressing the critical issue flagged in previous reviews. This ensures callers are aware of the checked exceptions thrown byjobClient.fetchData().
115-126: Verified: Set difference logic correctly excludes ignored runs.The filter condition
!ignoredRunIds.contains(run.id())properly excludes ignored runs from the result set, addressing the inverted logic issue flagged in previous reviews. The method now correctly returns runs that are infirstbut not insecond, while excluding any runs inignoredRunIds.
Comment on lines
+12
to
+17
| /** | ||
| * Returns an error message indicating that an HTTP Server Error occurred. | ||
| * @param code The status code of the HTTP Server Error response (must be between 500 and 599). | ||
| * @return The error message indicating that an HTTP Server Error occurred. | ||
| * @throws IllegalArgumentException If the entered code is not between 500 and 599 inclusive. | ||
| */ |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Good documentation; minor clarification suggestion.
The Javadoc is accurate and helpful for maintainability. One minor improvement: in the @param description, "between 500 and 599" could be interpreted as exclusive or inclusive. Consider clarifying with "from 500 to 599 inclusive" or "500-599 inclusive" to match the precision in the @throws clause.
📝 Suggested refinement
/**
* Returns an error message indicating that an HTTP Server Error occurred.
- * @param code The status code of the HTTP Server Error response (must be between 500 and 599).
+ * @param code The status code of the HTTP Server Error response (must be from 500 to 599 inclusive).
* @return The error message indicating that an HTTP Server Error occurred.
* @throws IllegalArgumentException If the entered code is not between 500 and 599 inclusive.
*/📝 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
| /** | |
| * Returns an error message indicating that an HTTP Server Error occurred. | |
| * @param code The status code of the HTTP Server Error response (must be between 500 and 599). | |
| * @return The error message indicating that an HTTP Server Error occurred. | |
| * @throws IllegalArgumentException If the entered code is not between 500 and 599 inclusive. | |
| */ | |
| /** | |
| * Returns an error message indicating that an HTTP Server Error occurred. | |
| * @param code The status code of the HTTP Server Error response (must be from 500 to 599 inclusive). | |
| * @return The error message indicating that an HTTP Server Error occurred. | |
| * @throws IllegalArgumentException If the entered code is not between 500 and 599 inclusive. | |
| */ |
🤖 Prompt for AI Agents
In
@src/main/java/com/example/github_workflow_tool/api/exceptions/ServerError.java
around lines 12 - 17, The Javadoc for the method in class ServerError uses
"between 500 and 599" which can be ambiguous; update the @param description for
the status code in ServerError to read "from 500 to 599 inclusive" (or "500–599
inclusive") so it clearly matches the @throws clause that requires the value to
be within that inclusive range.
Comment on lines
+37
to
+43
| /** | ||
| * Returns the name of the executable | ||
| * @return The name of the executable, specified by the "name" property in the env file. | ||
| */ | ||
| public String getAppName() { | ||
| return this.properties.getProperty("name"); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all usages of getAppName() to verify null handling
rg -n -A 3 -B 1 'getAppName\(\)' --type javaRepository: SerbanUntu/github-workflow-tool
Length of output: 1027
🏁 Script executed:
#!/bin/bash
# Examine EnvService class to understand initialization and validation
cat -n src/main/java/com/example/github_workflow_tool/cli/EnvService.javaRepository: SerbanUntu/github-workflow-tool
Length of output: 1830
🏁 Script executed:
#!/bin/bash
# Examine StorageService class context
cat -n src/main/java/com/example/github_workflow_tool/cli/StorageService.java | head -50Repository: SerbanUntu/github-workflow-tool
Length of output: 2513
Add validation for required "name" property in EnvService.
getAppName() will return null if the "name" property is missing from application.properties, causing a NullPointerException when StorageService calls path.resolve(this.envService.getAppName()) at line 41. Since the app name is required, validate and throw EnvException during EnvService initialization if the property is missing, similar to the check for the properties file itself.
🤖 Prompt for AI Agents
In @src/main/java/com/example/github_workflow_tool/cli/EnvService.java around
lines 37 - 43, EnvService currently returns null from getAppName() when the
"name" property is missing causing NPEs downstream (e.g.,
StorageService.path.resolve(this.envService.getAppName())). Validate the
presence of the "name" property during EnvService initialization (e.g., in the
constructor or init method) and if absent throw an EnvException with a clear
message; update getAppName() to either return the validated value or keep it
as-is but rely on the constructor check to guarantee non-null. Ensure the thrown
exception type is EnvException and reference EnvService and getAppName() so
callers (like StorageService) can assume a non-null app name.
Comment on lines
+12
to
+17
| /** | ||
| * Returns an error message indicating that an HTTP Client Error occurred. | ||
| * @param code The status code of the HTTP Client Error response (must be between 400 and 499). | ||
| * @return The error message indicating that an HTTP Client Error occurred. | ||
| * @throws IllegalArgumentException If the entered code is not between 400 and 499 inclusive. | ||
| */ |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM! Documentation is accurate and complete.
The Javadoc clearly describes the method's validation logic, parameters, return value, and exception behavior. While documenting private methods is less common, it's helpful for methods with validation logic like this one.
Optional: Consider more concise documentation
For a relatively straightforward validation method, the Javadoc could be slightly more concise:
- /**
- * Returns an error message indicating that an HTTP Client Error occurred.
- * @param code The status code of the HTTP Client Error response (must be between 400 and 499).
- * @return The error message indicating that an HTTP Client Error occurred.
- * @throws IllegalArgumentException If the entered code is not between 400 and 499 inclusive.
- */
+ /**
+ * Validates that code is a 4xx status and builds a user-friendly error message.
+ * @param code must be between 400 and 499 inclusive
+ * @throws IllegalArgumentException if code is outside the 4xx range
+ */However, the current documentation is perfectly acceptable and aligns with your PR's documentation goals.
📝 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
| /** | |
| * Returns an error message indicating that an HTTP Client Error occurred. | |
| * @param code The status code of the HTTP Client Error response (must be between 400 and 499). | |
| * @return The error message indicating that an HTTP Client Error occurred. | |
| * @throws IllegalArgumentException If the entered code is not between 400 and 499 inclusive. | |
| */ | |
| /** | |
| * Validates that code is a 4xx status and builds a user-friendly error message. | |
| * @param code must be between 400 and 499 inclusive | |
| * @throws IllegalArgumentException if code is outside the 4xx range | |
| */ |
🤖 Prompt for AI Agents
In
@src/main/java/com/example/github_workflow_tool/cli/exceptions/GenericClientError.java
around lines 12 - 17, No change required—the Javadoc for the method in
GenericClientError accurately documents validation, params, return, and
exception; if you want to tighten it further, shorten the comment to a single
concise sentence describing the method's purpose and validation or make it
package-private/private documentation briefer, but no functional code changes
are needed.
Comment on lines
8
to
10
| public TooFewArgumentsException(int numberOfArguments) { | ||
| super("Too few arguments provided to the command. Expected 2, got " + numberOfArguments + "."); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM! Clear and user-friendly exception message.
The constructor correctly formats a user-friendly error message. Optionally, consider adding defensive validation to ensure numberOfArguments < 2, which would prevent misleading messages if the exception is ever misused (e.g., instantiated with numberOfArguments = 3 would produce "Expected 2, got 3" for an exception named "TooFewArgumentsException").
🛡️ Optional defensive validation
public TooFewArgumentsException(int numberOfArguments) {
+ if (numberOfArguments >= 2) {
+ throw new IllegalArgumentException("numberOfArguments must be less than 2 for TooFewArgumentsException");
+ }
super("Too few arguments provided to the command. Expected 2, got " + numberOfArguments + ".");
}📝 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
| public TooFewArgumentsException(int numberOfArguments) { | |
| super("Too few arguments provided to the command. Expected 2, got " + numberOfArguments + "."); | |
| } | |
| public TooFewArgumentsException(int numberOfArguments) { | |
| if (numberOfArguments >= 2) { | |
| throw new IllegalArgumentException("numberOfArguments must be less than 2 for TooFewArgumentsException"); | |
| } | |
| super("Too few arguments provided to the command. Expected 2, got " + numberOfArguments + "."); | |
| } |
🤖 Prompt for AI Agents
In
@src/main/java/com/example/github_workflow_tool/cli/exceptions/TooFewArgumentsException.java
around lines 8 - 10, The TooFewArgumentsException constructor currently builds a
message from numberOfArguments; add defensive validation inside
TooFewArgumentsException(int numberOfArguments) to guard against misuse by
checking if numberOfArguments < 2 and if not either throw an
IllegalArgumentException (e.g., "TooFewArgumentsException must be constructed
with numberOfArguments < 2") or normalize the message to reflect a minimum of 2;
update the constructor logic to perform this check before calling super(...) so
misleading messages (like "Expected 2, got 3") cannot be produced.
Comment on lines
+24
to
+28
| @ParameterizedTest() | ||
| @CsvSource({"02,2", "32,32", "502,502"}) | ||
| public void formatStepNumberSingleDigitTest(String expected, int input) { | ||
| assertEquals(expected, sut.formatStepNumber(input)); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Misleading test method name.
The test method name formatStepNumberSingleDigitTest suggests it only tests single-digit inputs, but the CSV source includes multi-digit numbers (32, 502). Consider renaming to formatStepNumberTest or similar.
♻️ Proposed fix
- public void formatStepNumberSingleDigitTest(String expected, int input) {
+ public void formatStepNumberTest(String expected, int input) {
assertEquals(expected, sut.formatStepNumber(input));
}🤖 Prompt for AI Agents
In @src/test/java/com/example/github_workflow_tool/cli/CLIPrinterTests.java
around lines 24 - 28, The test method name formatStepNumberSingleDigitTest is
misleading because the CsvSource includes multi-digit inputs; rename the test
method to a neutral name like formatStepNumberTest (or
formatStepNumberParameterizedTest) so it accurately reflects that
sut.formatStepNumber is validated across single- and multi-digit values; update
the method declaration and any references to the old name accordingly.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:
- Around line 53-59: The current loop pairs steps by index which can misalign
when steps are added/removed; change the logic in DiffingService to match steps
by step.number(): build a map from jobBefore.steps() keyed by step.number(),
then for each JobStep stepAfter in jobAfter.steps() look up the corresponding
stepBefore by stepAfter.number() (or null if missing) and pass those to
compareSteps(runAfter, jobAfter, stepBefore, stepAfter); ensure you still handle
a null jobBefore and preserve existing behavior when numbers are absent or
duplicate by documenting/handling those edge cases.
In @src/main/java/com/example/github_workflow_tool/domain/events/Event.java:
- Around line 51-65: The compareTo and compareOptionalLongs implementations use
(int)(long subtraction) which can overflow; replace those subtractions with
Long.compare to preserve ordering for large IDs: update compareOptionalLongs to
return Long.compare(a.getAsLong(), b.getAsLong()) when both OptionalLongs are
present, and in compareTo replace (int)(this.runId - other.runId) with
Long.compare(this.runId, other.runId); keep existing empty-Optional behavior
(return 0) and leave timestamp.compareTo and getOrder comparisons as-is, then
feed these Long.compare results into getFirstNonZeroComparison.
In
@src/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.java:
- Around line 10-44: StepFailedEvent lacks an override of getOrder() for
deterministic sorting; add a protected int getOrder() override in the
StepFailedEvent class (same place as getColor()/getEventTag()) that returns the
same ordering value used by StepStartedEvent for failed-step priority so it
sorts consistently with other step events (match the numeric value from
StepStartedEvent's getOrder()).
In
@src/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.java:
- Around line 10-44: StepSucceededEvent is missing an override of getOrder()
which breaks deterministic sorting; add a protected int getOrder() override in
the StepSucceededEvent class that returns the same ordering value used for
succeeded events (match the value used by StepStartedEvent patterns in the
codebase) so that StepSucceededEvent sorts deterministically when timestamps are
equal; locate the class StepSucceededEvent and implement the getOrder() method
(signature protected int getOrder()) returning the appropriate integer constant
used for "succeeded" step events.
In
@src/test/java/com/example/github_workflow_tool/diffing/DiffingServiceTests.java:
- Around line 27-170: The setUp() fixture is very long; extract small builder
helpers to improve readability by creating private methods like makeStep(...)
returning a JobStep, makeJob(...) returning a Job, and makeRun(...) returning a
WorkflowRun, then replace the long inline new JobStep/new Job/new WorkflowRun
calls in setUp() with calls to those helpers (refer to the existing setUp(),
JobStep, Job, and WorkflowRun usages to locate where to replace).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
src/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/test/java/com/example/github_workflow_tool/diffing/DiffingServiceTests.java
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2026-01-07T12:51:57.721Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:101-109
Timestamp: 2026-01-07T12:51:57.721Z
Learning: In the github-workflow-tool project (Java), when a WorkflowRun has a status other than "waiting" or "requested" (i.e., when it reaches AFTER_QUEUEING state in the diffing logic), the startedAt() field is guaranteed to be non-null per the GitHub API specification.
Applied to files:
src/test/java/com/example/github_workflow_tool/diffing/DiffingServiceTests.java
📚 Learning: 2026-01-07T12:48:39.017Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:183-191
Timestamp: 2026-01-07T12:48:39.017Z
Learning: In Java files handling GitHub workflow API responses, assume that a step's completedAt field is non-null when the step's conclusion is set (e.g., 'success', 'failure'); the presence of a conclusion implies the step is completed. Update code to rely on completedAt being non-null in these cases and avoid false negatives from null checks. This guideline applies to all workflow step result handling across the project.
Applied to files:
src/test/java/com/example/github_workflow_tool/diffing/DiffingServiceTests.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.java
📚 Learning: 2026-01-07T12:49:09.426Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:170-178
Timestamp: 2026-01-07T12:49:09.426Z
Learning: In any Java code within the project, when modeling GitHub workflow steps, if a JobStep has a non-null conclusion (e.g., 'failure' or 'success'), the corresponding completedAt field is guaranteed to be non-null according to the GitHub API specification. Ensure your parsing and JSON mapping respect this invariant for all completed steps with a conclusion, and add nullability checks and appropriate null-safe handling in DiffingService and related types.
Applied to files:
src/test/java/com/example/github_workflow_tool/diffing/DiffingServiceTests.javasrc/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.java
📚 Learning: 2026-01-07T12:48:52.865Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:157-165
Timestamp: 2026-01-07T12:48:52.865Z
Learning: In the github-workflow-tool project (Java), when a JobStep has status "in_progress", the startedAt() field is guaranteed to be non-null per the GitHub API specification. Similarly, when a Job has status "in_progress", the startedAt() field is guaranteed to be non-null.
Applied to files:
src/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.java
📚 Learning: 2026-01-07T12:51:57.721Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java:101-109
Timestamp: 2026-01-07T12:51:57.721Z
Learning: In DiffingService.java, when evaluating a WorkflowRun, if the status is neither 'waiting' nor 'requested' (i.e., AFTER_QUEUEING in your diffing logic), you can rely on startedAt() being non-null as per the GitHub API spec. Do not perform null checks for startedAt in this code path; document this guarantee and access startedAt() directly, and optionally add a unit test asserting non-null in that path.
Applied to files:
src/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.java
📚 Learning: 2026-01-07T12:45:23.183Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.java:11-20
Timestamp: 2026-01-07T12:45:23.183Z
Learning: In the github-workflow-tool project (Java), fields in domain event classes (e.g., WorkflowQueuedEvent, JobEvent, StepEvent) and domain objects can be null per the GitHub API specification. Null validation should not be enforced on these fields as null is a valid state.
Applied to files:
src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java
📚 Learning: 2026-01-05T12:38:46.923Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/domain/Repository.java:3-3
Timestamp: 2026-01-05T12:38:46.923Z
Learning: In the SerbanUntu/github-workflow-tool repository, for Java source files under src/main/java, prefer using wildcard imports for java.util (import java.util.*;) instead of explicit imports for individual java.util classes to reduce visual clutter. Ensure this aligns with project conventions; if there are naming conflicts or readability concerns, consider switching to explicit imports for those cases.
Applied to files:
src/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.java
📚 Learning: 2026-01-05T12:53:43.895Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/domain/Repository.java:15-17
Timestamp: 2026-01-05T12:53:43.895Z
Learning: Guideline: In Java source files, boolean getters should follow the isXX naming pattern instead of getIsXX. For a boolean field (e.g., private boolean valid;), name the getter isValid() rather than getIsValid(). If the field is already named isXxx, keep the generated getter as isXxx (e.g., private boolean isValid; getter should be isValid()). Refactor any getIsXxx usages to isXxx, update overrides/equals/hashCode if they rely on property names, and adjust any frameworks or serialization logic that rely on bean property names. Apply consistently across the src/main/java tree, not just the specific file.
Applied to files:
src/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.java
📚 Learning: 2026-01-05T13:57:46.893Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 4
File: src/main/java/com/example/github_workflow_tool/GithubWorkflowToolApplication.java:21-23
Timestamp: 2026-01-05T13:57:46.893Z
Learning: In Java projects, ensure that user-facing errors do not display stack traces. Replace stack traces with concise, user-friendly error messages. Log full stack traces internally (e.g., using a logger) and present a generic message to the user. Apply this guideline to all user-facing error handling across the application.
Applied to files:
src/main/java/com/example/github_workflow_tool/diffing/DiffingService.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.java
📚 Learning: 2026-01-07T12:45:23.183Z
Learnt from: SerbanUntu
Repo: SerbanUntu/github-workflow-tool PR: 14
File: src/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.java:11-20
Timestamp: 2026-01-07T12:45:23.183Z
Learning: Do not enforce non-null validation for fields in domain event classes and domain objects in the github-workflow-tool project. According to the GitHub API specification, fields like those in WorkflowQueuedEvent, JobEvent, and StepEvent can be null and represent valid states. Update code reviews to allow null values where the API permits, avoid extra null checks, and adjust tests and validation rules to reflect nullable fields in the domain/events package.
Applied to files:
src/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/Event.javasrc/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.javasrc/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.java
🧬 Code graph analysis (1)
src/main/java/com/example/github_workflow_tool/domain/events/Event.java (1)
src/main/java/com/example/github_workflow_tool/cli/CLIPrinter.java (1)
CLIPrinter(15-167)
🔇 Additional comments (7)
src/main/java/com/example/github_workflow_tool/domain/events/StepStartedEvent.java (1)
10-56: LGTM: Well-structured event class with complete ordering support.The implementation correctly overrides all necessary methods including
getOrder()for deterministic event sorting when timestamps collide.src/main/java/com/example/github_workflow_tool/domain/events/JobFinishedEvent.java (1)
1-55: LGTM!The
JobFinishedEventimplementation is clean and consistent with the event hierarchy. The order value (5) correctly places it after job-started events (2), and the yellow color appropriately signals completion.src/main/java/com/example/github_workflow_tool/domain/events/StepEvent.java (1)
105-116:jobIdexcluded fromequals()andhashCode()— verify this is intentional.The
jobIdfield is used for comparison ordering viagetJobIdForComparison()but is excluded fromequals()andhashCode(). If two different jobs (differentjobId) happen to share the samestepName,stepNumber, andjobNamewithin the same run, they would be considered equal.If this is intentional (e.g.,
jobNameis guaranteed unique per run), consider adding a comment. Otherwise, includejobIdin equality checks.🔎 Proposed fix if jobId should be included
@Override public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; if (!super.equals(o)) return false; StepEvent stepEvent = (StepEvent) o; - return stepNumber == stepEvent.stepNumber && Objects.equals(stepName, stepEvent.stepName) && Objects.equals(jobName, stepEvent.jobName); + return jobId == stepEvent.jobId && stepNumber == stepEvent.stepNumber && Objects.equals(stepName, stepEvent.stepName) && Objects.equals(jobName, stepEvent.jobName); } @Override public int hashCode() { - return Objects.hash(super.hashCode(), stepName, stepNumber, jobName); + return Objects.hash(super.hashCode(), jobId, stepName, stepNumber, jobName); }src/main/java/com/example/github_workflow_tool/domain/events/JobEvent.java (1)
88-99:jobIdexcluded fromequals()andhashCode()— same concern asStepEvent.Similar to
StepEvent, thejobIdfield is used for comparison but excluded from equality checks. If two jobs share the samejobNameandworkflowNamewithin a run (e.g., matrix builds), they would be considered equal despite having differentjobIdvalues.🔎 Proposed fix if jobId should be included
@Override public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; if (!super.equals(o)) return false; JobEvent jobEvent = (JobEvent) o; - return Objects.equals(jobName, jobEvent.jobName) && Objects.equals(workflowName, jobEvent.workflowName); + return jobId == jobEvent.jobId && Objects.equals(jobName, jobEvent.jobName) && Objects.equals(workflowName, jobEvent.workflowName); } @Override public int hashCode() { - return Objects.hash(super.hashCode(), jobName, workflowName); + return Objects.hash(super.hashCode(), jobId, jobName, workflowName); }src/main/java/com/example/github_workflow_tool/domain/events/WorkflowQueuedEvent.java (1)
1-126: LGTM!The
WorkflowQueuedEventimplementation is well-structured. The order value (1) correctly positions queued events before job-started (2) and other events in the timeline. Theequals/hashCodeimplementation properly includesworkflowName.src/main/java/com/example/github_workflow_tool/domain/events/JobStartedEvent.java (1)
1-56: LGTM!Clean implementation following the established event hierarchy pattern. The order value (2) correctly positions job-started events after workflow-queued (1) but before step events (4).
src/main/java/com/example/github_workflow_tool/domain/events/Event.java (1)
128-155: Well-designed formatting infrastructure.The
getEventPrefix()andappendBranchAndCommitSha()methods provide a clean, reusable foundation for consistent event rendering across all subclasses. The ANSI color handling and padding logic are cleanly encapsulated.
Comment on lines
+53
to
+59
| for (int i = 0; i < jobAfter.steps().size(); i++) { | ||
| JobStep stepAfter = jobAfter.steps().get(i); | ||
| JobStep stepBefore = (jobBefore == null || jobBefore.steps().size() <= i) | ||
| ? null | ||
| : jobBefore.steps().get(i); | ||
| events.addAll(compareSteps(runAfter, jobAfter, stepBefore, stepAfter)); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider matching steps by number field for robustness.
Steps are matched by list index, which assumes stable ordering. If the workflow definition changes between polling intervals (steps added/removed), this could incorrectly pair different steps. Matching by step.number() would be more robust against workflow definition changes.
♻️ Alternative step matching approach
- for (int i = 0; i < jobAfter.steps().size(); i++) {
- JobStep stepAfter = jobAfter.steps().get(i);
- JobStep stepBefore = (jobBefore == null || jobBefore.steps().size() <= i)
- ? null
- : jobBefore.steps().get(i);
+ Map<Integer, JobStep> stepBeforeByNumber = jobBefore == null
+ ? Collections.emptyMap()
+ : jobBefore.steps().stream()
+ .collect(Collectors.toMap(JobStep::number, s -> s));
+
+ for (JobStep stepAfter : jobAfter.steps()) {
+ JobStep stepBefore = stepBeforeByNumber.get(stepAfter.number());
events.addAll(compareSteps(runAfter, jobAfter, stepBefore, stepAfter));
}📝 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
| for (int i = 0; i < jobAfter.steps().size(); i++) { | |
| JobStep stepAfter = jobAfter.steps().get(i); | |
| JobStep stepBefore = (jobBefore == null || jobBefore.steps().size() <= i) | |
| ? null | |
| : jobBefore.steps().get(i); | |
| events.addAll(compareSteps(runAfter, jobAfter, stepBefore, stepAfter)); | |
| } | |
| Map<Integer, JobStep> stepBeforeByNumber = jobBefore == null | |
| ? Collections.emptyMap() | |
| : jobBefore.steps().stream() | |
| .collect(Collectors.toMap(JobStep::number, s -> s)); | |
| for (JobStep stepAfter : jobAfter.steps()) { | |
| JobStep stepBefore = stepBeforeByNumber.get(stepAfter.number()); | |
| events.addAll(compareSteps(runAfter, jobAfter, stepBefore, stepAfter)); | |
| } |
🤖 Prompt for AI Agents
In @src/main/java/com/example/github_workflow_tool/diffing/DiffingService.java
around lines 53 - 59, The current loop pairs steps by index which can misalign
when steps are added/removed; change the logic in DiffingService to match steps
by step.number(): build a map from jobBefore.steps() keyed by step.number(),
then for each JobStep stepAfter in jobAfter.steps() look up the corresponding
stepBefore by stepAfter.number() (or null if missing) and pass those to
compareSteps(runAfter, jobAfter, stepBefore, stepAfter); ensure you still handle
a null jobBefore and preserve existing behavior when numbers are absent or
duplicate by documenting/handling those edge cases.
Comment on lines
+51
to
+65
| private int compareOptionalLongs(OptionalLong a, OptionalLong b) { | ||
| if (a.isEmpty() || b.isEmpty()) return 0; | ||
| return (int) (a.getAsLong() - b.getAsLong()); | ||
| } | ||
|
|
||
| @Override | ||
| public int compareTo(Event other) { | ||
| return getFirstNonZeroComparison( | ||
| this.timestamp.compareTo(other.timestamp), | ||
| (int) (this.runId - other.runId), | ||
| compareOptionalLongs(this.getJobIdForComparison(), other.getJobIdForComparison()), | ||
| compareOptionalLongs(this.getStepNumberForComparison(), other.getStepNumberForComparison()), | ||
| this.getOrder() - other.getOrder() | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Integer overflow risk in compareTo when comparing long values.
Lines 53 and 60 cast long subtraction results to int, which can overflow for large values. GitHub run IDs are large numbers (e.g., 12345678901234567) that could trigger this. This violates the Comparable contract and may cause incorrect event ordering.
🔧 Proposed fix using Long.compare()
private int compareOptionalLongs(OptionalLong a, OptionalLong b) {
if (a.isEmpty() || b.isEmpty()) return 0;
- return (int) (a.getAsLong() - b.getAsLong());
+ return Long.compare(a.getAsLong(), b.getAsLong());
}
@Override
public int compareTo(Event other) {
return getFirstNonZeroComparison(
this.timestamp.compareTo(other.timestamp),
- (int) (this.runId - other.runId),
+ Long.compare(this.runId, other.runId),
compareOptionalLongs(this.getJobIdForComparison(), other.getJobIdForComparison()),
compareOptionalLongs(this.getStepNumberForComparison(), other.getStepNumberForComparison()),
this.getOrder() - other.getOrder()
);
}🤖 Prompt for AI Agents
In @src/main/java/com/example/github_workflow_tool/domain/events/Event.java
around lines 51 - 65, The compareTo and compareOptionalLongs implementations use
(int)(long subtraction) which can overflow; replace those subtractions with
Long.compare to preserve ordering for large IDs: update compareOptionalLongs to
return Long.compare(a.getAsLong(), b.getAsLong()) when both OptionalLongs are
present, and in compareTo replace (int)(this.runId - other.runId) with
Long.compare(this.runId, other.runId); keep existing empty-Optional behavior
(return 0) and leave timestamp.compareTo and getOrder comparisons as-is, then
feed these Long.compare results into getFirstNonZeroComparison.
Comment on lines
+10
to
+44
| public class StepFailedEvent extends StepEvent { | ||
|
|
||
| public StepFailedEvent( | ||
| Instant timestamp, | ||
| String branchName, | ||
| String commitSha, | ||
| long runId, | ||
| long jobId, | ||
| String stepName, | ||
| int stepNumber, | ||
| String jobName | ||
| ) { | ||
| super(timestamp, branchName, commitSha, runId, jobId, stepName, stepNumber, jobName); | ||
| } | ||
|
|
||
| /** | ||
| * Getter for the ANSI color for this event tag | ||
| * | ||
| * @return The ANSI color for this event tag | ||
| */ | ||
| @Override | ||
| protected Color getColor() { | ||
| return Color.RED; | ||
| } | ||
|
|
||
| /** | ||
| * Getter for the text of the event tag | ||
| * | ||
| * @return The text of the event tag for this event | ||
| */ | ||
| @Override | ||
| protected String getEventTag() { | ||
| return "FAILED"; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Add getOrder() override for deterministic event sorting.
StepStartedEvent overrides getOrder() to ensure deterministic sorting when multiple events share the same timestamp. StepFailedEvent should follow the same pattern to maintain consistent ordering behavior across all step event types.
🔢 Suggested implementation
protected String getEventTag() {
return "FAILED";
}
+
+ /**
+ * Used for determining which event to print first,
+ * when there are multiple events that happened at the same instant.
+ * An event with a lower order is printed before an event with a higher order with the same timestamp.
+ *
+ * @return The order of the event.
+ */
+ @Override
+ protected int getOrder() {
+ return 4;
+ }
}📝 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
| public class StepFailedEvent extends StepEvent { | |
| public StepFailedEvent( | |
| Instant timestamp, | |
| String branchName, | |
| String commitSha, | |
| long runId, | |
| long jobId, | |
| String stepName, | |
| int stepNumber, | |
| String jobName | |
| ) { | |
| super(timestamp, branchName, commitSha, runId, jobId, stepName, stepNumber, jobName); | |
| } | |
| /** | |
| * Getter for the ANSI color for this event tag | |
| * | |
| * @return The ANSI color for this event tag | |
| */ | |
| @Override | |
| protected Color getColor() { | |
| return Color.RED; | |
| } | |
| /** | |
| * Getter for the text of the event tag | |
| * | |
| * @return The text of the event tag for this event | |
| */ | |
| @Override | |
| protected String getEventTag() { | |
| return "FAILED"; | |
| } | |
| } | |
| public class StepFailedEvent extends StepEvent { | |
| public StepFailedEvent( | |
| Instant timestamp, | |
| String branchName, | |
| String commitSha, | |
| long runId, | |
| long jobId, | |
| String stepName, | |
| int stepNumber, | |
| String jobName | |
| ) { | |
| super(timestamp, branchName, commitSha, runId, jobId, stepName, stepNumber, jobName); | |
| } | |
| /** | |
| * Getter for the ANSI color for this event tag | |
| * | |
| * @return The ANSI color for this event tag | |
| */ | |
| @Override | |
| protected Color getColor() { | |
| return Color.RED; | |
| } | |
| /** | |
| * Getter for the text of the event tag | |
| * | |
| * @return The text of the event tag for this event | |
| */ | |
| @Override | |
| protected String getEventTag() { | |
| return "FAILED"; | |
| } | |
| /** | |
| * Used for determining which event to print first, | |
| * when there are multiple events that happened at the same instant. | |
| * An event with a lower order is printed before an event with a higher order with the same timestamp. | |
| * | |
| * @return The order of the event. | |
| */ | |
| @Override | |
| protected int getOrder() { | |
| return 4; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
@src/main/java/com/example/github_workflow_tool/domain/events/StepFailedEvent.java
around lines 10 - 44, StepFailedEvent lacks an override of getOrder() for
deterministic sorting; add a protected int getOrder() override in the
StepFailedEvent class (same place as getColor()/getEventTag()) that returns the
same ordering value used by StepStartedEvent for failed-step priority so it
sorts consistently with other step events (match the numeric value from
StepStartedEvent's getOrder()).
Comment on lines
+10
to
+44
| public class StepSucceededEvent extends StepEvent { | ||
|
|
||
| public StepSucceededEvent( | ||
| Instant timestamp, | ||
| String branchName, | ||
| String commitSha, | ||
| long runId, | ||
| long jobId, | ||
| String stepName, | ||
| int stepNumber, | ||
| String jobName | ||
| ) { | ||
| super(timestamp, branchName, commitSha, runId, jobId, stepName, stepNumber, jobName); | ||
| } | ||
|
|
||
| /** | ||
| * Getter for the ANSI color for this event tag | ||
| * | ||
| * @return The ANSI color for this event tag | ||
| */ | ||
| @Override | ||
| protected Color getColor() { | ||
| return Color.GREEN; | ||
| } | ||
|
|
||
| /** | ||
| * Getter for the text of the event tag | ||
| * | ||
| * @return The text of the event tag for this event | ||
| */ | ||
| @Override | ||
| protected String getEventTag() { | ||
| return "SUCCEEDED"; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Add getOrder() override for deterministic event sorting.
StepStartedEvent overrides getOrder() to ensure deterministic sorting when multiple events share the same timestamp. StepSucceededEvent should follow the same pattern to maintain consistent ordering behavior across all step event types.
🔢 Suggested implementation
protected String getEventTag() {
return "SUCCEEDED";
}
+
+ /**
+ * Used for determining which event to print first,
+ * when there are multiple events that happened at the same instant.
+ * An event with a lower order is printed before an event with a higher order with the same timestamp.
+ *
+ * @return The order of the event.
+ */
+ @Override
+ protected int getOrder() {
+ return 5;
+ }
}📝 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
| public class StepSucceededEvent extends StepEvent { | |
| public StepSucceededEvent( | |
| Instant timestamp, | |
| String branchName, | |
| String commitSha, | |
| long runId, | |
| long jobId, | |
| String stepName, | |
| int stepNumber, | |
| String jobName | |
| ) { | |
| super(timestamp, branchName, commitSha, runId, jobId, stepName, stepNumber, jobName); | |
| } | |
| /** | |
| * Getter for the ANSI color for this event tag | |
| * | |
| * @return The ANSI color for this event tag | |
| */ | |
| @Override | |
| protected Color getColor() { | |
| return Color.GREEN; | |
| } | |
| /** | |
| * Getter for the text of the event tag | |
| * | |
| * @return The text of the event tag for this event | |
| */ | |
| @Override | |
| protected String getEventTag() { | |
| return "SUCCEEDED"; | |
| } | |
| } | |
| public class StepSucceededEvent extends StepEvent { | |
| public StepSucceededEvent( | |
| Instant timestamp, | |
| String branchName, | |
| String commitSha, | |
| long runId, | |
| long jobId, | |
| String stepName, | |
| int stepNumber, | |
| String jobName | |
| ) { | |
| super(timestamp, branchName, commitSha, runId, jobId, stepName, stepNumber, jobName); | |
| } | |
| /** | |
| * Getter for the ANSI color for this event tag | |
| * | |
| * @return The ANSI color for this event tag | |
| */ | |
| @Override | |
| protected Color getColor() { | |
| return Color.GREEN; | |
| } | |
| /** | |
| * Getter for the text of the event tag | |
| * | |
| * @return The text of the event tag for this event | |
| */ | |
| @Override | |
| protected String getEventTag() { | |
| return "SUCCEEDED"; | |
| } | |
| /** | |
| * Used for determining which event to print first, | |
| * when there are multiple events that happened at the same instant. | |
| * An event with a lower order is printed before an event with a higher order with the same timestamp. | |
| * | |
| * @return The order of the event. | |
| */ | |
| @Override | |
| protected int getOrder() { | |
| return 5; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
@src/main/java/com/example/github_workflow_tool/domain/events/StepSucceededEvent.java
around lines 10 - 44, StepSucceededEvent is missing an override of getOrder()
which breaks deterministic sorting; add a protected int getOrder() override in
the StepSucceededEvent class that returns the same ordering value used for
succeeded events (match the value used by StepStartedEvent patterns in the
codebase) so that StepSucceededEvent sorts deterministically when timestamps are
equal; locate the class StepSucceededEvent and implement the getOrder() method
(signature protected int getOrder()) returning the appropriate integer constant
used for "succeeded" step events.
Comment on lines
+27
to
+170
| @BeforeEach | ||
| public void setUp() { | ||
| JobStep step1Before = new JobStep( | ||
| "Set up Job", | ||
| "completed", | ||
| "success", | ||
| 1, | ||
| makeInstant(10, 0, 0), | ||
| makeInstant(10, 1, 0) | ||
| ); | ||
| JobStep step2Before = new JobStep( | ||
| "Finish Job", | ||
| "in_progress", | ||
| null, | ||
| 2, | ||
| makeInstant(10, 1, 0), | ||
| null | ||
| ); | ||
| JobStep step1After = new JobStep( | ||
| "Set up Job", | ||
| "completed", | ||
| "success", | ||
| 1, | ||
| makeInstant(10, 0, 0), | ||
| makeInstant(10, 1, 0) | ||
| ); | ||
| JobStep step2After = new JobStep( | ||
| "Finish Job", | ||
| "completed", | ||
| "success", | ||
| 2, | ||
| makeInstant(10, 1, 0), | ||
| makeInstant(10, 2, 1) | ||
| ); | ||
| JobStep step3After = new JobStep( | ||
| "Set up Job", | ||
| "completed", | ||
| "success", | ||
| 1, | ||
| makeInstant(10, 2, 1), | ||
| makeInstant(10, 3, 3) | ||
| ); | ||
| JobStep step4After = new JobStep( | ||
| "Finish Job", | ||
| "completed", | ||
| "success", | ||
| 2, | ||
| makeInstant(10, 3, 3), | ||
| makeInstant(10, 4, 4) | ||
| ); | ||
| JobStep step5After = new JobStep( | ||
| "Set up Job", | ||
| "in_progress", | ||
| null, | ||
| 1, | ||
| makeInstant(11, 0, 0), | ||
| null | ||
| ); | ||
| Job job1Before = new Job( | ||
| 10L, | ||
| 1L, | ||
| "asdf", | ||
| "in_progress", | ||
| null, | ||
| "Run tests", | ||
| makeInstant(10, 0, 0), | ||
| null, | ||
| List.of(step1Before, step2Before) | ||
| ); | ||
| Job job1After = new Job( | ||
| 10L, | ||
| 1L, | ||
| "asdf", | ||
| "completed", | ||
| "success", | ||
| "Run tests", | ||
| makeInstant(10, 0, 0), | ||
| makeInstant(10, 2, 1), | ||
| List.of(step1After, step2After) | ||
| ); | ||
| Job job2After = new Job( | ||
| 11L, | ||
| 1L, | ||
| "asdf", | ||
| "completed", | ||
| "success", | ||
| "Build executable", | ||
| makeInstant(10, 2, 1), | ||
| makeInstant(10, 4, 4), | ||
| List.of(step3After, step4After) | ||
| ); | ||
| Job job3After = new Job( | ||
| 12L, | ||
| 2L, | ||
| "asdf", | ||
| "in_progress", | ||
| null, | ||
| "Run tests", | ||
| makeInstant(11, 0, 0), | ||
| null, | ||
| List.of(step5After) | ||
| ); | ||
| WorkflowRun run1Before = new WorkflowRun( | ||
| 1L, | ||
| 100L, | ||
| "asdf", | ||
| "CI", | ||
| "dev", | ||
| "in_progress", | ||
| null, | ||
| makeInstant(9, 59, 0), | ||
| makeInstant(10, 0, 0), | ||
| makeInstant(10, 0, 0) | ||
| ); | ||
| WorkflowRun run1After = new WorkflowRun( | ||
| 1L, | ||
| 100L, | ||
| "asdf", | ||
| "CI", | ||
| "dev", | ||
| "completed", | ||
| "success", | ||
| makeInstant(9, 59, 0), | ||
| makeInstant(10, 4, 4), | ||
| makeInstant(10, 0, 0) | ||
| ); | ||
| WorkflowRun run2After = new WorkflowRun( | ||
| 2L, | ||
| 100L, | ||
| "asdf", | ||
| "CI", | ||
| "dev", | ||
| "in_progress", | ||
| null, | ||
| makeInstant(10, 59, 0), | ||
| makeInstant(11, 0, 0), | ||
| makeInstant(11, 0, 0) | ||
| ); | ||
| WorkflowRunData run1DataBefore = new WorkflowRunData(run1Before, Map.of(10L, job1Before)); | ||
| WorkflowRunData run1DataAfter = new WorkflowRunData(run1After, Map.of(10L, job1After, 11L, job2After)); | ||
| WorkflowRunData run2DataAfter = new WorkflowRunData(run2After, Map.of(12L, job3After)); | ||
| before = Map.of(1L, run1DataBefore); | ||
| after = Map.of(1L, run1DataAfter, 2L, run2DataAfter); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting fixture builders for improved readability.
The setUp() method spans 140+ lines. Extracting helper methods for creating JobStep, Job, and WorkflowRun objects would improve readability and maintainability.
♻️ Example helper methods
private JobStep makeStep(String name, String status, String conclusion, int number,
Instant startedAt, Instant completedAt) {
return new JobStep(name, status, conclusion, number, startedAt, completedAt);
}
private Job makeJob(Long id, Long runId, String sha, String status, String conclusion,
String name, Instant startedAt, Instant completedAt, List<JobStep> steps) {
return new Job(id, runId, sha, status, conclusion, name, startedAt, completedAt, steps);
}
private WorkflowRun makeRun(Long id, Long workflowId, String sha, String name, String branch,
String status, String conclusion, Instant createdAt,
Instant updatedAt, Instant startedAt) {
return new WorkflowRun(id, workflowId, sha, name, branch, status, conclusion,
createdAt, updatedAt, startedAt);
}🤖 Prompt for AI Agents
In
@src/test/java/com/example/github_workflow_tool/diffing/DiffingServiceTests.java
around lines 27 - 170, The setUp() fixture is very long; extract small builder
helpers to improve readability by creating private methods like makeStep(...)
returning a JobStep, makeJob(...) returning a Job, and makeRun(...) returning a
WorkflowRun, then replace the long inline new JobStep/new Job/new WorkflowRun
calls in setUp() with calls to those helpers (refer to the existing setUp(),
JobStep, Job, and WorkflowRun usages to locate where to replace).
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.
Changes:
Todos:
Strings when there are no new eventsinstallDisttaskCloses #5, #9, #11, #13