Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the Gemini log parser to handle Gemini CLI JSONL (typed per-line JSON entries) and generate consistent markdown output via shared log parsing/rendering helpers.
Changes:
- Refactors
parse_gemini_log.cjsto parse Gemini JSONL, transform entries into the canonical schema, and reuse shared markdown generation helpers. - Adds
transformGeminiEntries()to normalize Gemini-specific entry types (init/message/tool_use/tool_result) into the shared format. - Introduces a new Vitest suite covering parsing, delta merging, tool rendering, and stats extraction.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| actions/setup/js/parse_gemini_log.cjs | Reworks Gemini parser to ingest JSONL, normalize entries, and render markdown via shared helpers (conversation + info sections). |
| actions/setup/js/parse_gemini_log.test.cjs | Adds unit tests validating JSONL parsing behavior, tool formatting, delta merge behavior, and stats extraction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cache_read_input_tokens: stats.cached || 0, | ||
| }, | ||
| duration_ms: stats.duration_ms || 0, | ||
| num_turns: stats.tool_calls || 0, |
There was a problem hiding this comment.
num_turns is being populated from stats.tool_calls, but generateInformationSection() renders num_turns as Turns. Tool call count is not the same as conversation turns, so this will produce misleading summaries. Consider omitting num_turns for Gemini (or mapping it from an actual turns field if Gemini provides one) and, if desired, render tool call count via an additionalInfoCallback instead.
| num_turns: stats.tool_calls || 0, |
| }, | ||
| }); | ||
| } else if (raw.type === "tool_result") { | ||
| const output = typeof raw.output === "string" ? raw.output : JSON.stringify(raw.output || ""); |
There was a problem hiding this comment.
When raw.output is not a string, JSON.stringify(raw.output || "") will incorrectly coerce valid falsy outputs like 0, false, or "" into "", and undefined becomes "\"\"". Use a nullish check (e.g., raw.output == null ? "" : JSON.stringify(raw.output)) so primitives are preserved and missing output stays empty.
| const output = typeof raw.output === "string" ? raw.output : JSON.stringify(raw.output || ""); | |
| const output = | |
| typeof raw.output === "string" | |
| ? raw.output | |
| : raw.output == null | |
| ? "" | |
| : JSON.stringify(raw.output); |
|
🧪 Smoke Temporary ID is now testing temporary ID functionality... |
|
🚀 Smoke Gemini MISSION COMPLETE! Gemini has spoken. ✨ Smoke test completed. Build failed. Reported results to PR #17605. |
|
🧪 Smoke Project is now testing project operations... |
|
📰 BREAKING: Smoke Copilot ARM64 is now investigating this pull request. Sources say the story is developing... |
|
✅ Smoke Project completed successfully. All project operations validated. |
|
Smoke Test Results for Gemini:
Overall Status: FAIL
|
|
🦾 ARM64 Smoke Test Results — §22265960089 Arch:
Overall:
|
There was a problem hiding this comment.
Quick review from the ARM64 smoke test agent 🦾 — the refactoring of parseGeminiLog to use a two-pass JSONL approach (parse all entries first, then transform) is clean and aligns well with the shared generateConversationMarkdown pattern used by other engine parsers. Two minor positive observations left inline. LGTM overall.
📰 BREAKING: Report filed by Smoke Copilot ARM64
| for (const line of logContent.split("\n")) { | ||
| const trimmed = line.trim(); | ||
| if (!trimmed) { | ||
| if (!trimmed || !trimmed.startsWith("{")) { |
There was a problem hiding this comment.
Nice defensive check — filtering lines that don't start with { avoids unnecessary JSON.parse attempts on non-object lines, which is a good micro-optimization for large log files. 👍
| markdown += lastResponse + "\n\n"; | ||
| if (rawEntries.length === 0) { | ||
| return { | ||
| markdown: "## 🤖 Gemini\n\nLog format not recognized as Gemini JSONL.\n\n", |
There was a problem hiding this comment.
Providing a friendly error message when the log format isn't recognized is a great UX touch. Makes debugging much easier when a user accidentally points this at a non-Gemini log file. ✨
|
📰 VERDICT: Smoke Copilot ARM64 has concluded. All systems operational. This is a developing story. 🎤 |
|
@copilot configure Gemini to allow reading outside the workspace See errors https://github.com/github/gh-aw/actions/runs/22265960083/job/64412439749#step:37:1 |
|
📰 BREAKING: Smoke Copilot ARM64 is now investigating this pull request. Sources say the story is developing... |
|
🧪 Smoke Temporary ID is now testing temporary ID functionality... |
|
🚀 Smoke Gemini MISSION COMPLETE! Gemini has spoken. ✨ Smoke test failed due to Go version mismatch. Results posted to PR #17605. |
|
🧪 Smoke Project is now testing project operations... |
|
✅ Smoke Project completed successfully. All project operations validated. |
Smoke Test Results
Overall Status: FAIL
|
|
🦾 ARM64 Smoke Test §22266358873 — PASS ✅ Architecture:
Overall: PASS (10/11 — Serena not provisioned)
|
There was a problem hiding this comment.
Reviewed from ARM64 (aarch64) 🦾 — PR refactors Gemini log parsing to use shared log_parser_shared.cjs helpers, reducing duplication and aligning output format with other engine parsers. The transformGeminiEntries pipeline cleanly maps Gemini JSONL types to the canonical format. Two inline comments left on the import expansion and delta-merging logic.
📰 BREAKING: Report filed by Smoke Copilot ARM64
| /// <reference types="@actions/github-script" /> | ||
|
|
||
| const { createEngineLogParser } = require("./log_parser_shared.cjs"); | ||
| const { createEngineLogParser, generateConversationMarkdown, generateInformationSection, formatInitializationSummary, formatToolUse } = require("./log_parser_shared.cjs"); |
There was a problem hiding this comment.
Good refactoring — importing generateConversationMarkdown, generateInformationSection, formatInitializationSummary, and formatToolUse from the shared module eliminates duplicated formatting logic and ensures consistent output across all engine parsers.
| logEntries, | ||
| mcpFailures: [], | ||
| maxTurnsHit: false, | ||
| }; |
There was a problem hiding this comment.
The streaming delta merging logic here correctly handles Gemini's chunked assistant messages. The guard on entry.message.content.length === 1 ensures only single-content entries are merged, preventing accidental data loss. Consider adding a comment clarifying why multi-content entries are excluded from merging.
|
📰 VERDICT: Smoke Copilot ARM64 has concluded. All systems operational. This is a developing story. 🎤 |
…pe definitions
- Fix 3 subjective language issues across 3 spec files:
- engine-review-summary.md: remove 'production-ready' qualifier,
replace 'Easy to understand and follow' with technical description
- go-type-patterns.md: replace 'Easy refactoring' with factual description
- serena-tools-quick-reference.md: replace '✓ Perfect' with '✓ Complete'
- Add TypeScript Type Definitions section to dev.md Safe Outputs System
documenting handler-factory.d.ts, safe-outputs.d.ts, safe-outputs-config.d.ts,
and github-script.d.ts from PR #17605
- Bump dev.md version to 2.7
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
No description provided.