Conversation
…ata format Improve error handling in the frontend to show specific messages for different HTTP status codes (404, 400, 500+) instead of generic errors. Add _transform_json_entry_to_timeline() method to convert JSON log data into the format expected by the frontend (step_count, total_duration_ms, steps array with proper fields). Update tests to verify the new response format structure.
…onse Add event_type, action, repository, sender, pr, success, and error fields to _format_timeline_data() response. These fields are needed by the frontend to render complete workflow context in the timeline modal.
Collaborator
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds JSON→timeline transformation, a new per-step log retrieval API and controller method, frontend step-modal rendering and error handling, CSS for step/details UI, and expanded tests/fixtures validating time-windowed step log behavior and stricter malformed-data errors. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (log_viewer.js)
participant API as FastAPI app (app.py)
participant Controller as LogViewerController (log_viewer.py)
participant Storage as JSON Log Storage
Browser->>API: GET /logs/api/workflow-steps?hookId=...
API->>Controller: get_workflow_steps_json(hook_id)
Controller->>Storage: fetch raw JSON entry for hook_id
Storage-->>Controller: raw entry (timing, workflow_steps, token_spend, ...)
Note over Controller: _transform_json_entry_to_timeline(entry, hook_id)\nvalidate timing & workflow_steps\ncompute start_time/total_duration_ms\nconvert workflow_steps → ordered steps\nderive per-step fields (timestamp, duration_ms, relative_time_ms, error)
alt malformed entry
Controller-->>API: raise 500 { "errorDetail": "Malformed log entry" }
API-->>Browser: HTTP 500 JSON
else OK
Controller-->>API: 200 JSON timeline (hook_id, start_time, total_duration_ms, step_count, steps, ...)
API-->>Browser: HTTP 200 JSON
end
Browser->>API: GET /logs/api/step-logs/{hook_id}/{step_name}
API->>Controller: get_step_logs(hook_id, step_name)
Controller->>Controller: locate step, compute window (duration or default)\ncap results to _MAX_STEP_LOGS
Controller->>Storage: filter logs by time window & hook_id
Storage-->>Controller: matching log entries
Controller-->>API: 200 JSON { step: metadata, logs: [...], log_count }
API-->>Browser: 200 JSON
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@webhook_server/web/log_viewer.py`:
- Around line 556-563: The docstring for get_workflow_steps_json is missing the
new public context fields returned by the function; update its Returns (or
description of the returned dictionary) to include event_type, action,
repository, sender, pr, success, and error with brief descriptions and types
(e.g., strings/booleans/null), so the API contract matches the actual response
structure and prevents client misuse; locate get_workflow_steps_json and append
these fields to the existing list of returned keys and their short explanations.
- Around line 642-654: The steps_list append is hardcoding "task_id" and
"task_type" to None which discards grouping metadata; update the append in the
code that builds steps_list (the block using step_timestamp, step_name, message,
level, repository, event_type, pr_number, step_status, step_duration_ms,
step_error) to pull task metadata from the parsed step_data (e.g. use
step_data.get("task_id") and step_data.get("task_type") or similar) and default
to None if missing so existing task_id/task_type in JSON are preserved for UI
grouping.
- Around line 606-616: The code currently defaults missing timing/workflow
fields to empty values which masks malformed logs; instead, in the log parsing
function that processes "entry" (uses variables timing, workflow_steps,
start_time, total_duration_ms, pr_number) validate that entry contains non-empty
"timing" and "workflow_steps" and that timing contains "started_at" and
"duration_ms"; if any required field is missing or invalid, raise a clear
exception (e.g., ValueError) or return an error result so callers fail fast and
surface the malformed log rather than silently using 0/{} defaults.
In `@webhook_server/web/static/js/log_viewer.js`:
- Around line 842-872: The 404 branch discards the parsed server errorDetail;
update the response handler inside the .then(async (response) => { ... }) block
to use the parsed errorDetail (if present) when calling showFlowModalError and
logging for the 404 case (refer to hookId and showFlowModalError), e.g. build a
message = errorDetail || "No workflow data found for this hook" and use
console.log/console.error with that message so server-provided context is
surfaced to operators.
myakove-bot
added
commented-coderabbitai[bot]
lgtm-coderabbitai[bot]
and removed
changes-requested-coderabbitai[bot]
labels
…line - Document new context fields (event_type, action, repository, sender, pr, success, error) in get_workflow_steps_json docstring - Fail fast on missing timing/workflow_steps instead of defaulting to empty values, raising HTTPException with clear error messages - Preserve task metadata (task_id, task_type) from JSON steps for grouping - Surface server-provided detail on 404 errors in JavaScript modal
myakove-bot
added
size/XL
and removed
commented-rnetser
commented-coderabbitai[bot]
lgtm-coderabbitai[bot]
size/L
labels
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@webhook_server/web/log_viewer.py`:
- Around line 631-634: The line computing total_duration_ms should not coerce
falsy zero values; replace the fallback pattern and use the validated field
directly by assigning total_duration_ms = timing["duration_ms"] (leave
start_time = timing["started_at"] as-is) so explicit 0 durations are preserved;
update the code around the timing dict usage in log_viewer.py where
total_duration_ms is computed.
- Around line 689-703: pr_info is currently set with a fallback to {} earlier
(via entry.get("pr") or {}), so returning "pr": pr_info if pr_info else None is
redundant; to return None when PR info is absent, use the original lookup value
(entry.get("pr")) in the return or change the earlier assignment to preserve
None when absent. Update the code to either (a) use "pr": entry.get("pr") in the
returned dict, or (b) set pr_info = entry.get("pr") (no "or {}") and keep "pr":
pr_info if pr_info else None so absence yields None; reference the pr_info
variable and entry.get("pr") to locate the change.
Fixes two issues identified in code review:
- Remove 'or 0' fallback that masked valid zero-duration webhooks
- Remove redundant truthiness check on pr_info by using entry.get("pr") directly
- Update mock_logger to use spec=logging.Logger with realistic attributes - Add error field to sample_json_webhook_data when success is False - Document JSONL format in create_json_log_file fixture
Collaborator
Author
|
/build-and-push-container |
Collaborator
|
New container for ghcr.io/myk-org/github-webhook-server:pr-982 published |
Collaborator
Author
|
/build-and-push-container |
Collaborator
|
New container for ghcr.io/myk-org/github-webhook-server:pr-982 published |
Collaborator
|
/lgtm |
Collaborator
Author
|
/build-and-push-container |
Collaborator
|
New container for ghcr.io/myk-org/github-webhook-server:pr-982 published |
… detail Step metadata (status, duration, error) is already displayed via renderStepDetails - the "No log entries found during this step's execution" message below it was redundant and confusing.
Collaborator
Author
|
/build-and-push-container |
Collaborator
|
New container for ghcr.io/myk-org/github-webhook-server:pr-982 published |
- Add step_details field to workflow step objects so the frontend can display all step metadata (not just name/status/duration) - Set task_id to step_name for proper step grouping in the UI - Render step_details as formatted log entries showing start time, metadata fields, and completion status with duration
Collaborator
Author
|
/build-and-push-container |
Collaborator
|
New container for ghcr.io/myk-org/github-webhook-server:pr-982 published |
Setting task_id to step_name caused each step to become a collapsed group with 1 item, requiring extra clicks to see content. Revert to task_id=None so steps render as immediately visible ungrouped items.
…eps" This reverts commit bd28382.
The step-logs endpoint was returning empty logs because _stream_log_entries prioritizes JSON summary files which consumed the entire 50K entry budget before text .log files were reached. - Add _stream_text_log_entries that reads only .log files (not JSON) - Use it in get_step_logs to find actual processing logs matching the hook_id within the step's time window - Revert task_id back to step_name for proper step identification - Add comprehensive tests for log viewer coverage (90.50%)
Collaborator
Author
|
/build-and-push-container |
Collaborator
|
New container for ghcr.io/myk-org/github-webhook-server:pr-982 published |
Collaborator
Author
|
/verified |
Collaborator
|
/lgtm |
Collaborator
|
Successfully removed PR tag: ghcr.io/myk-org/github-webhook-server:pr-982. |
Collaborator
|
New container for ghcr.io/myk-org/github-webhook-server:latest published |
This was referenced Mar 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
Summary
_format_timeline_data()response that are required by the frontend timeline modalChanges
Adds 7 new fields to the workflow timeline API response:
event_type- GitHub event type (e.g., "pull_request", "check_run")action- Event action (e.g., "opened", "synchronize")repository- Repository name (owner/repo)sender- GitHub username who triggered the eventpr- Pull request info (number, title, etc.)success- Boolean indicating if processing succeedederror- Error message if processing failedSummary by CodeRabbit
New Features
Improvements
Bug Fixes
Security
Tests