Skip to content

[code-simplifier] refactor: extract hasValidPrefix boolean in exception type extraction (#25972 follow-up)#25989

Merged
pelikhan merged 1 commit intomainfrom
code-simplifier/exception-type-extraction-2026-04-13-2417b0e21f285e56
Apr 13, 2026
Merged

[code-simplifier] refactor: extract hasValidPrefix boolean in exception type extraction (#25972 follow-up)#25989
pelikhan merged 1 commit intomainfrom
code-simplifier/exception-type-extraction-2026-04-13-2417b0e21f285e56

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

This PR simplifies recently modified code from PR #25972 to improve clarity while preserving all functionality.

Files Simplified

  • actions/setup/js/send_otlp_span.cjs — extracted hasValidPrefix boolean to remove duplicated condition logic

Improvement Made

Before:

const exceptionType = colonIdx > 0 && colonIdx < 64 && /^[a-z_][a-z0-9_.]*$/i.test(prefix) ? `gh-aw.\$\{prefix.toLowerCase()}` : "gh-aw.AgentError";
const exceptionMessage = (colonIdx > 0 && exceptionType !== "gh-aw.AgentError" ? msg.slice(colonIdx + 1).trim() : msg).slice(0, MAX_ATTR_VALUE_LENGTH);

After:

const hasValidPrefix = colonIdx > 0 && colonIdx < 64 && /^[a-z_][a-z0-9_.]*$/i.test(prefix);
const exceptionType = hasValidPrefix ? `gh-aw.\$\{prefix.toLowerCase()}` : "gh-aw.AgentError";
const exceptionMessage = (hasValidPrefix ? msg.slice(colonIdx + 1).trim() : msg).slice(0, MAX_ATTR_VALUE_LENGTH);

The exceptionMessage line previously re-checked colonIdx > 0 && exceptionType !== "gh-aw.AgentError". The colonIdx > 0 part was redundant — if exceptionType !== "gh-aw.AgentError" is true, colonIdx > 0 is necessarily also true. More importantly, checking the string value of exceptionType to determine message extraction is indirect; using the same hasValidPrefix predicate makes the relationship explicit and avoids the implicit coupling between the two variables.

Changes Based On

Testing

  • ✅ All exception type extraction tests pass (extracts exception.type from colon-prefixed error messages, falls back to gh-aw.AgentError when message has no colon prefix, etc.)
  • ✅ All event attribute sanitization tests pass
  • ✅ 156 of 159 tests pass (3 pre-existing failures unrelated to this change: GITHUB_RUN_ATTEMPT env var and version file environment issues)
  • ✅ No functional changes — behavior is identical

Review Focus

Please verify:

  • The hasValidPrefix boolean correctly captures the original condition
  • exceptionType and exceptionMessage remain semantically equivalent to the original code

Automated by Code Simplifier Agent — analyzing code from the last 24 hours

Generated by Code Simplifier · ● 1.7M ·

  • expires on Apr 14, 2026, 6:31 AM UTC

Replaces the duplicated condition logic in sendJobConclusionSpan with
an explicit hasValidPrefix boolean. This removes the redundant
colonIdx > 0 check in the exceptionMessage line (it was already
implied by the exceptionType !== 'gh-aw.AgentError' test), and makes
it immediately clear that both exceptionType and exceptionMessage
derive from the same predicate.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor Author

Hey @github-actions[bot] 👋 — great work simplifying the exception-type extraction logic in send_otlp_span.cjs! Extracting hasValidPrefix removes the implicit coupling between exceptionType and exceptionMessage and makes the predicate reuse explicit — a clear readability win.

One item to address before this is ready for maintainer review:

  • No test code in the diff — While the PR body confirms all existing tests pass (✅), the diff itself contains no changes to send_otlp_span.test.cjs or any adjacent test file. For a refactor like this, a brief regression test or an explicit assertion that the hasValidPrefix path is exercised for both true and false branches would make the intent clear and guard against future regressions. Without any test-side evidence in the diff, reviewers have to take the description at face value.

If you'd like a hand, you can assign this prompt to your coding agent:

In actions/setup/js/send_otlp_span.test.cjs, verify (or add) explicit test coverage for the hasValidPrefix refactor introduced in send_otlp_span.cjs around line 785.

Specifically ensure the following scenarios are covered:
1. A colon-prefixed message with a valid prefix (e.g. "push_to_pull_request_branch: details") → exceptionType should be "gh-aw.push_to_pull_request_branch" and exceptionMessage should be trimmed to "details".
2. A message with no colon → exceptionType should be "gh-aw.AgentError" and exceptionMessage should be the full message.
3. A message where colonIdx >= 64 → exceptionType should be "gh-aw.AgentError" and exceptionMessage should be the full message.
4. A message where the prefix fails the /^[a-z_][a-z0-9_.]*$/i regex → same fallback behaviour.

Use the existing test patterns in send_otlp_span.test.cjs (describe/it blocks, mock setup, etc.).

Generated by Contribution Check · ● 3.2M ·

@pelikhan pelikhan marked this pull request as ready for review April 13, 2026 13:43
Copilot AI review requested due to automatic review settings April 13, 2026 13:43
@pelikhan pelikhan merged commit a7bd203 into main Apr 13, 2026
34 checks passed
@pelikhan pelikhan deleted the code-simplifier/exception-type-extraction-2026-04-13-2417b0e21f285e56 branch April 13, 2026 13:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Simplifies exception type/message extraction for OTLP exception span events by factoring the shared “valid prefix” predicate into a dedicated boolean, preserving existing behavior while improving readability.

Changes:

  • Extracted hasValidPrefix to avoid duplicated/indirect conditional logic.
  • Updated exceptionType and exceptionMessage derivation to use the shared predicate consistently.
Show a summary per file
File Description
actions/setup/js/send_otlp_span.cjs Refactors exception prefix validation into hasValidPrefix and reuses it for both exception.type and exception.message extraction.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants