Skip to content

Add progressive NOTE disclosure to comment-memory footer#27848

Merged
pelikhan merged 5 commits intomainfrom
copilot/update-comment-memory-handler
Apr 22, 2026
Merged

Add progressive NOTE disclosure to comment-memory footer#27848
pelikhan merged 5 commits intomainfrom
copilot/update-comment-memory-handler

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 22, 2026

  • Inspect current comment-memory body builder and related tests
  • Run baseline validation before changes (make agent-finish) and record pre-existing failures
  • Add a progressive-disclosure NOTE alert in the comment-memory handler near the generated footer, outside the memory XML island
  • Update/extend JavaScript tests to validate the new note appears in the managed comment body
  • Run targeted JS tests and formatting/lint checks
  • Run make agent-finish before final commit and capture results
  • Run parallel validation and address any actionable findings
  • Create/update PR with final checklist state

Copilot AI and others added 2 commits April 22, 2026 13:58
Comment thread actions/setup/js/comment_memory.cjs Outdated
// that happen to contain a matching comment-memory tag.
const MANAGED_COMMENT_PROVENANCE_MARKER = "<!-- gh-aw-agentic-workflow:";
const MANAGED_COMMENT_HEADER = "### Comment Memory";
const MANAGED_COMMENT_DISCLOSURE_NOTE = [
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot refactor text to .md file in actions/setup/md

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Implemented in a7287ea: the disclosure NOTE text is now in /home/runner/work/gh-aw/gh-aw/actions/setup/md/comment_memory_disclosure_note.md, and comment_memory.cjs loads it from that md template.

Screenshot: N/A (no UI change; this is a text-template refactor only).

@pelikhan pelikhan marked this pull request as ready for review April 22, 2026 14:05
Copilot AI review requested due to automatic review settings April 22, 2026 14:05
Copilot AI requested a review from pelikhan April 22, 2026 14:07
@github-actions github-actions Bot mentioned this pull request Apr 22, 2026
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

Adds a progressive-disclosure NOTE block to the managed “comment memory” comment body so readers understand what the comment is for, while keeping the memory XML island intact and the generated footer last.

Changes:

  • Introduces a new MANAGED_COMMENT_DISCLOSURE_NOTE markdown blockquote (NOTE + <details>/<summary>) inserted before the generated footer.
  • Extends the comment_memory unit test to assert the NOTE is present and ordered between the memory block and the footer.
Show a summary per file
File Description
actions/setup/js/comment_memory.cjs Appends a disclosure NOTE block to managed comment bodies when includeFooter is enabled.
actions/setup/js/comment_memory.test.cjs Updates assertions to validate the NOTE content and placement relative to the memory block and footer.

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: 2

Comment thread actions/setup/js/comment_memory.cjs Outdated
Comment on lines 59 to 60
body += "\n\n" + MANAGED_COMMENT_DISCLOSURE_NOTE;
body += "\n\n" + generateFooterWithMessages(workflowName, runUrl, workflowSource, workflowSourceURL, triggeringIssueNumber, triggeringPRNumber, undefined, historyUrl).trimEnd();
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

Appending MANAGED_COMMENT_DISCLOSURE_NOTE increases the fixed overhead of managed comments. Since enforceCommentLimits caps bodies at 65,536 chars, this reduces the maximum size available for the user-provided memory block and could cause previously-valid large memories to start failing with E006. Consider reserving space for the note+footer (e.g., truncate/compact the memory content when close to the limit, or emit a more specific error that accounts for footer/note overhead).

Copilot uses AI. Check for mistakes.
Comment thread actions/setup/js/comment_memory.cjs Outdated
Comment on lines +21 to +30
const MANAGED_COMMENT_DISCLOSURE_NOTE = [
"> [!NOTE]",
"> This comment is managed by comment memory.",
">",
"> <details>",
"> <summary>What this comment does</summary>",
">",
"> It stores persistent context for this thread in the `<gh-aw-comment-memory>` block at the top of this comment.",
"> Edit only the text in that block; workflow metadata and the footer are regenerated automatically.",
"> </details>",
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

MANAGED_COMMENT_DISCLOSURE_NOTE hard-codes the tag name (<gh-aw-comment-memory>) even though the actual tag is defined by COMMENT_MEMORY_TAG. This duplicates a source of truth and could become incorrect if the tag constant ever changes. Consider constructing this part of the note from COMMENT_MEMORY_TAG (or describing it without spelling the literal tag).

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100

Excellent test quality

Metric Value
New/modified tests analyzed 1 (modified existing it() block)
✅ Design tests (behavioral contracts) 1 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 1 (100%)
Duplicate test clusters 0
Test inflation detected No (7 test lines / 12 prod lines = 0.58 ratio)
🚨 Coding-guideline violations None

Test Classification Details

Test File Classification Issues Detected
"builds managed comment body with footer when enabled" (modified) actions/setup/js/comment_memory.test.cjs:47 ✅ Design None

Analysis

The PR adds 4 assertions to the existing "builds managed comment body with footer when enabled" test in comment_memory.test.cjs. No new it() blocks were introduced — only targeted assertions extending an existing test.

New assertions (all behavioral):

  1. expect(body).toContain("> [!NOTE]") — verifies the disclosure NOTE block is present in the generated output
  2. expect(body).toContain("> <summary>What this comment does</summary>") — verifies the summary text is included
  3. expect(memoryEndIdx).toBeLessThan(noteIdx) — enforces structural ordering: </gh-aw-comment-memory> must come before the NOTE block
  4. expect(noteIdx).toBeLessThan(footerIdx) — enforces structural ordering: NOTE block must come before the "Generated by" footer

The ordering assertions (#3 and #4) are particularly strong — they verify a structural invariant (memory → note → footer) that would catch a regression if the block placement logic were accidentally reordered. This goes beyond a simple content-presence check.


Language Support

Tests analyzed:

  • 🟨 JavaScript (*.test.cjs): 1 test block modified (vitest)
  • 🐹 Go (*_test.go): No changes

Verdict

Check passed. 0% of modified test assertions are implementation tests (threshold: 30%). The test changes directly enforce the behavioral contract of the new MANAGED_COMMENT_DISCLOSURE_NOTE feature.

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

References: §24782856587

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

Copy link
Copy Markdown
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.

✅ Test Quality Sentinel: 100/100. Test quality is excellent — 0% of new test assertions are implementation tests (threshold: 30%). The 4 added assertions verify both content presence and structural ordering invariants for the new disclosure NOTE feature.

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot review all comments

Comment thread actions/setup/js/comment_memory.cjs Outdated
// that happen to contain a matching comment-memory tag.
const MANAGED_COMMENT_PROVENANCE_MARKER = "<!-- gh-aw-agentic-workflow:";
const MANAGED_COMMENT_HEADER = "### Comment Memory";
const MANAGED_COMMENT_DISCLOSURE_NOTE = renderTemplateFromFile(path.join(__dirname, "../md/comment_memory_disclosure_note.md"), {});
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot render lazily, move into handler

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Implemented in 9f41193 (with lazy-render change introduced in 96a8647): disclosure note rendering is now lazy and performed in the handler path, with cached reuse per handler instance.

Screenshot: N/A (no UI change).

Copilot AI and others added 2 commits April 22, 2026 14:37
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8d98ac82-6815-4400-b775-96d88cbe20a5

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

Copilot AI commented Apr 22, 2026

@copilot review all comments

Reviewed all comments and addressed the actionable items in 96a8647 and 9f41193.

Screenshot: N/A (no UI change).

Copilot AI requested a review from pelikhan April 22, 2026 14:44
@pelikhan pelikhan merged commit 892724d into main Apr 22, 2026
12 checks passed
@pelikhan pelikhan deleted the copilot/update-comment-memory-handler branch April 22, 2026 14:45
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.

3 participants