Skip to content

[jsweep] Clean add_reaction_and_edit_comment.cjs#30062

Merged
pelikhan merged 2 commits intomainfrom
jsweep/add-reaction-and-edit-comment-2384da54ee6903ea
May 4, 2026
Merged

[jsweep] Clean add_reaction_and_edit_comment.cjs#30062
pelikhan merged 2 commits intomainfrom
jsweep/add-reaction-and-edit-comment-2384da54ee6903ea

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot commented May 4, 2026

Summary

Cleaned actions/setup/js/add_reaction_and_edit_comment.cjs by removing code duplication.

Context

github-script context (uses github, core, context globals)

Changes

Removed duplicated code:

  • addReaction() — identical implementation existed in add_reaction.cjs; now imported from there
  • addDiscussionReaction() — identical implementation existed in add_reaction.cjs; now imported from there
  • DISCUSSION_REACTION_MAP constant — identical to REACTION_MAP in add_reaction.cjs; removed in favour of the imported functions using their own map

The duplicated functions were byte-for-byte identical to their counterparts in add_reaction.cjs, with no logic difference. They are now imported and re-exported so the public API of this module is unchanged.

Test Improvements

Added 2 new test cases (33 → 35 tests):

  • should add lock notice for issue_comment event when GH_AW_LOCK_FOR_AGENT=true — the lock-notice condition covers both issues and issue_comment events but only the issues case was tested
  • should not add lock notice for discussion events when GH_AW_LOCK_FOR_AGENT=true — confirms lock notice is suppressed for non-issue event types

Validation ✅

  • Formatting: npm run format:cjs
  • Linting: npm run lint:cjs
  • Type checking: npm run typecheck ✓ (pre-existing unrelated error in aw_context.cjs only)
  • Tests: npx vitest run add_reaction_and_edit_comment — 35/35 passed ✓

Generated by jsweep - JavaScript Unbloater · ● 2.1M ·

  • expires on May 6, 2026, 5:00 AM UTC

…d_edit_comment.cjs

- Import addReaction and addDiscussionReaction from add_reaction.cjs
  instead of duplicating the identical implementations
- Remove the duplicated DISCUSSION_REACTION_MAP (identical to REACTION_MAP)
- Add 2 new test cases: lock notice for issue_comment events and
  no lock notice for discussion events (GH_AW_LOCK_FOR_AGENT=true)
- Tests: 35 passed (was 33)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review May 4, 2026 13:54
Copilot AI review requested due to automatic review settings May 4, 2026 13:54
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

Refactors add_reaction_and_edit_comment.cjs to reuse the shared reaction helpers from add_reaction.cjs, reducing duplicated implementation while preserving the module’s exported API.

Changes:

  • Replaced locally duplicated reaction helper functions in add_reaction_and_edit_comment.cjs with imports from add_reaction.cjs.
  • Removed the duplicated discussion reaction map now that the shared helper owns that mapping.
  • Added lock-notice tests for issue_comment and discussion comment creation paths.
Show a summary per file
File Description
actions/setup/js/add_reaction_and_edit_comment.cjs Swaps duplicated local reaction helpers for shared imports and keeps existing exports intact.
actions/setup/js/add_reaction_and_edit_comment.test.cjs Adds coverage for lock-notice behavior on issue_comment and discussion events.

Copilot's findings

Tip

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

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

@github-actions github-actions Bot mentioned this pull request May 4, 2026
@github-actions
Copy link
Copy Markdown
Contributor Author

github-actions Bot commented May 4, 2026

🧪 Test Quality Sentinel Report

Test Quality Score: 75/100

⚠️ Acceptable — with suggestions

Metric Value
New/modified tests analyzed 2
✅ Design tests (behavioral contracts) 2 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 1 (50%)
Duplicate test clusters 0
Test inflation detected ✅ YES — 24:1 ratio (test: +24 lines, prod: +1 line)
🚨 Coding-guideline violations None

Test Classification Details

Test File Classification Issues Detected
should add lock notice for issue_comment event when GH_AW_LOCK_FOR_AGENT=true add_reaction_and_edit_comment.test.cjs:506 ✅ Design Happy-path only; no negative/error assertion
should not add lock notice for discussion events when GH_AW_LOCK_FOR_AGENT=true add_reaction_and_edit_comment.test.cjs:517 ✅ Design Covers negative case well

Flagged Items — Notes

⚠️ Test Inflation: add_reaction_and_edit_comment.test.cjs

Ratio: 24 lines added in test vs. 1 line added in production (24:1, threshold: 2:1).

This is expected when backfilling missing test coverage for existing behavior, which appears to be the case here (the production change is a cleanup/sweep with no new logic). The tests themselves are valuable and cover real behavioral contracts. The inflation flag is noted but not a quality concern in this context.


Language Support

Tests analyzed:

  • 🟨 JavaScript (*.test.cjs): 2 tests (vitest)

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). Both tests verify observable behavioral contracts — that the lock notice is conditionally added based on event type. The test inflation flag is benign (adding coverage for existing behavior during a cleanup PR).


📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

🧪 Test quality analysis by Test Quality Sentinel · ● 390.8K ·

Copy link
Copy Markdown
Contributor Author

@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.

✅ Test Quality Sentinel: 75/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). Both new tests verify real behavioral contracts around the GH_AW_LOCK_FOR_AGENT lock notice feature.

@pelikhan
Copy link
Copy Markdown
Collaborator

pelikhan commented May 4, 2026

…le to Record

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b0267898-a1b6-4083-902c-5f3a467fd0ce

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented May 4, 2026

@copilot fix https://github.com/github/gh-aw/actions/runs/25323086685/job/74236789987

Fixed in 003f098. Added an isRecord() type predicate helper that properly narrows unknown → Record<string, unknown>, replacing the broken JSDoc cast. The root cause was that Prettier (TypeScript parser) strips parentheses from /** @type {T} */ (identifier) casts, leaving /** @type {T} */ identifier which TypeScript rejects for object → Record<string, unknown> assignments.

Copilot AI requested a review from pelikhan May 4, 2026 18:07
@pelikhan pelikhan merged commit 8152c80 into main May 4, 2026
@pelikhan pelikhan deleted the jsweep/add-reaction-and-edit-comment-2384da54ee6903ea branch May 4, 2026 18:16
Copilot AI added a commit that referenced this pull request May 4, 2026
* refactor(js): remove duplicated reaction helpers from add_reaction_and_edit_comment.cjs

- Import addReaction and addDiscussionReaction from add_reaction.cjs
  instead of duplicating the identical implementations
- Remove the duplicated DISCUSSION_REACTION_MAP (identical to REACTION_MAP)
- Add 2 new test cases: lock notice for issue_comment events and
  no lock notice for discussion events (GH_AW_LOCK_FOR_AGENT=true)
- Tests: 35 passed (was 33)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(js): fix TypeScript error in aw_context.cjs - object not assignable to Record

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b0267898-a1b6-4083-902c-5f3a467fd0ce

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
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.

3 participants