Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { MarkdownRenderer } from "@features/editor/components/MarkdownRenderer";
import { formatSignalReportSummaryMarkdown } from "@features/inbox/utils/formatSignalReportSummaryMarkdown";
import { Box } from "@radix-ui/themes";

interface SignalReportSummaryMarkdownProps {
Expand All @@ -23,7 +24,9 @@ export function SignalReportSummaryMarkdown({
variant,
pending,
}: SignalReportSummaryMarkdownProps) {
const raw = content?.trim() ? content : fallback;
const raw = formatSignalReportSummaryMarkdown(
content?.trim() ? content : fallback,
);

/** List rows: only the first line (before first newline); CSS still caps visual lines. */
const listMarkdown = raw.split(/\r?\n/)[0] ?? "";
Comment on lines +27 to 32
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 The listMarkdown computation splits raw on the first newline, but after formatting, raw now begins with `What's happening:

, so listMarkdownbecomes just"What's happening:". The list` variant then renders only the bold header (e.g. "What's happening:") with no actual summary content — a regression from the previous behaviour where the full inline text was visible. Using the pre-formatted content for the list split preserves the original single-line text for the list preview.

Suggested change
const raw = formatSignalReportSummaryMarkdown(
content?.trim() ? content : fallback,
);
/** List rows: only the first line (before first newline); CSS still caps visual lines. */
const listMarkdown = raw.split(/\r?\n/)[0] ?? "";
const rawContent = content?.trim() ? content : fallback;
const raw = formatSignalReportSummaryMarkdown(rawContent);
/** List rows: only the first line (before first newline); CSS still caps visual lines. */
const listMarkdown = rawContent.split(/\r?\n/)[0] ?? "";
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/inbox/components/utils/SignalReportSummaryMarkdown.tsx
Line: 27-32

Comment:
The `listMarkdown` computation splits `raw` on the first newline, but after formatting, `raw` now begins with `**What's happening:**`, so `listMarkdown` becomes just `"**What's happening:**"`. The `list` variant then renders only the bold header (e.g. "What's happening:") with no actual summary content — a regression from the previous behaviour where the full inline text was visible. Using the pre-formatted content for the list split preserves the original single-line text for the list preview.

```suggestion
  const rawContent = content?.trim() ? content : fallback;
  const raw = formatSignalReportSummaryMarkdown(rawContent);

  /** List rows: only the first line (before first newline); CSS still caps visual lines. */
  const listMarkdown = rawContent.split(/\r?\n/)[0] ?? "";
```

How can I resolve this? If you propose a fix, please make it concise.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { describe, expect, it } from "vitest";
import { formatSignalReportSummaryMarkdown } from "./formatSignalReportSummaryMarkdown";

describe("formatSignalReportSummaryMarkdown", () => {
it("puts section body text on a new line after the header", () => {
const input =
"**What's happening:** Error tracking issue keyed on `app:dashboard_query`.";
expect(formatSignalReportSummaryMarkdown(input)).toBe(
"**What's happening:**\n\nError tracking issue keyed on `app:dashboard_query`.",
);
});

it("separates consecutive section headers onto their own lines", () => {
const input =
"**What's happening:** Users hit rate limits. **Root cause:** All four rate limiters are contended. **How to resolve:** Reduce blocking.";
expect(formatSignalReportSummaryMarkdown(input)).toBe(
"**What's happening:**\n\nUsers hit rate limits.\n\n**Root cause:**\n\nAll four rate limiters are contended.\n\n**How to resolve:**\n\nReduce blocking.",
);
});

it("separates a section header from preceding intro text", () => {
const input =
"Users on busy orgs are hitting hard limits. **What's happening:** Error tracking issue.";
expect(formatSignalReportSummaryMarkdown(input)).toBe(
"Users on busy orgs are hitting hard limits.\n\n**What's happening:**\n\nError tracking issue.",
);
});

it("leaves content without section headers unchanged", () => {
const input = "Plain summary with no structured sections.";
expect(formatSignalReportSummaryMarkdown(input)).toBe(input);
});
});
Comment on lines +4 to +33
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 All four cases share the same (input, expected) shape and should be collapsed into a single it.each table, per the team's preference for parameterised tests.

Context Used: Do not attempt to comment on incorrect alphabetica... (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/inbox/utils/formatSignalReportSummaryMarkdown.test.ts
Line: 4-33

Comment:
All four cases share the same `(input, expected)` shape and should be collapsed into a single `it.each` table, per the team's preference for parameterised tests.

**Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
const SIGNAL_SUMMARY_SECTION_HEADERS = [
"What's happening",
"Root cause",
"How to resolve",
] as const;

function escapeRegExp(value: string): string {
return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
}

/**
* Inserts line breaks around signal report summary section headers so each
* label and its body render on separate lines (matches agent output like
* `**What's happening:** text`).
*/
export function formatSignalReportSummaryMarkdown(content: string): string {
let result = content;

for (const header of SIGNAL_SUMMARY_SECTION_HEADERS) {
const escaped = escapeRegExp(header);
const boldHeaderPattern = `\\*\\*${escaped}:\\*\\*`;

result = result.replace(
new RegExp(`([^\\n])\\s*(${boldHeaderPattern})`, "gi"),
"$1\n\n$2",
);

result = result.replace(
new RegExp(`(${boldHeaderPattern})\\s+`, "gi"),
"$1\n\n",
);
}

return result;
}
Loading