Skip to content

Conversation

@lightninglu10
Copy link
Contributor

@lightninglu10 lightninglu10 commented Jun 18, 2025

Normalize PR description indentation

This PR adds a helper to remove common leading whitespace from multiline text and applies it to the extracted PR description, ensuring Markdown renders correctly without unintended code blocks.

Key Changes:

  • Added normalizeIndentation function in ai-client.ts
  • Replaced trim with normalizeIndentation on prDescription
  • Updated existing test to match normalized output

Review Notes:

  • Verify helper behavior on edge cases (empty lines, mixed indentation)
  • Ensure no regression in other text extraction logic

@lightninglu10 lightninglu10 self-assigned this Jun 18, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

❇️ CodePress Review Summary

👋 Hey team,

Overall the changes look solid, but I spotted 1 must-fix issue and left 2 helpful notes inline.

Here's the quick rundown:

✅ Decision: APPROVE
Changes are focused, well-tested, and address Markdown formatting issues without introducing blocking concerns.

🚧 Needs a bit of love

The review highlights that the new indentation‐normalizing helper in src/ai-client.ts is not exported, which prevents it from being unit tested in isolation. Exporting this function would enable more precise test coverage and improve maintainability by allowing targeted tests against its logic.

ℹ️ Other thoughts

Several observations point out that using a blanket .trim() call can inadvertently remove intentional blank lines at the beginning or end of content. To avoid losing meaningful whitespace—such as in code or quoted blocks—it may be preferable to use methods like .trimEnd(), apply trimming only to lines composed entirely of whitespace, or introduce an optional flag to preserve edge blank lines.

return line.slice(minIndent);
});

return normalizedLines.join("\n").trim();
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 OPTIONAL: Calling .trim() here will drop any leading or trailing blank lines—even if they were intentional.
If preserving blank lines at the edges is important, consider using .trimEnd() or only trimming lines consisting entirely of whitespace.

);
const prDescription = prDescriptionMatch
? prDescriptionMatch[1].trim()
? normalizeIndentation(prDescriptionMatch[1])
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 OPTIONAL: The normalizeIndentation helper calls .trim() on the joined lines, which will remove any intentional leading or trailing blank lines. If you need to preserve blank lines (for example in code or quote blocks), consider using a more targeted approach (e.g., trimEnd() only) or exposing a flag to opt out of trimming at the edges.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@lightninglu10 lightninglu10 merged commit d991ab4 into main Jun 18, 2025
3 checks passed
@lightninglu10 lightninglu10 deleted the fix-pr-description-formatting branch June 18, 2025 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant