Improve comment-memory config test coverage

[Copilot]

comment-memory config tests only exercised workflow parsing for boolean/null cases, leaving map-form fields and direct parser branches uncovered.

• Map-form coverage

  • Adds assertions for max, memory-id, target, target-repo, allowed-repos, and footer.

• Invalid config coverage

  • Verifies invalid max and footer values disable CommentMemory.

• Parser branch coverage

  • Adds direct unit coverage for parseCommentMemoryConfigValue with nil, false, and true.

config := compiler.parseCommentMemoryConfigValue(true)

assert.Equal(t, &CommentMemoryConfig{
    BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")},
    MemoryID: "default",
}, config)

• Fixes [testify-expert] Improve Test Quality: pkg/workflow/comment_memory_config_test.go #51621

---

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 13 AIC · ⌖ 5.34 AIC · ⊞ 8.5K · ◷
_{Comment /souschef to run again}

[github-actions]

PR Triage

• Category: test (test-quality improvement for comment_memory_config_test.go, per testify-expert)
• Risk: low
• Priority score: 18/100 (impact 6, urgency 4, quality 8)
• Recommended action: defer

Notes: Draft PR, 0 files changed / 0 additions so far — agent has not yet pushed any code (description says work is starting). CI check copilot is still in_progress. Revisit once the draft has substantive changes and is marked ready for review.

Generated by 🔧 PR Triage Agent · auto · 30.1 AIC · ⌖ 2.51 AIC · ⊞ 7.8K · ◷

[Copilot]

Pull request overview

Expands comment-memory configuration tests to cover parser branches and map-form behavior.

Changes:

• Tests all map-form fields and defaults.
• Covers invalid max and footer values.
• Directly tests boolean and nil parser branches.

Show a summary per file

File	Description
pkg/workflow/comment_memory_config_test.go	Adds comprehensive comment-memory configuration coverage.

Review details

Tip

Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 1/1 changed files
• Comments generated: 0
• Review effort level: Balanced

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

🧪 Test quality analysis by Test Quality Sentinel

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

Warning

Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.

What happened

The threat detection engine failed to produce results.

Review the workflow run logs for details.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• api.individual.githubcopilot.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "api.individual.githubcopilot.com"

See Network Configuration for more information.

🔎 Code quality review by PR Code Quality Reviewer

[github-actions]

✅ Ponytail Reviewer completed successfully!

Lean already. Ship.

Generated by Ponytail Reviewer for #51622

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

🏗️ ADR gate enforced by Design Decision Gate 🏗️

[github-actions]

The test improvements look solid. Replacing the boolean flag with *CommentMemoryConfig makes assertions more precise and self-documenting. The new test cases for invalid fields and bool/nil config values fill meaningful coverage gaps. No issues found.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 16.1 AIC · ⌖ 6.79 AIC · ⊞ 5.4K

[github-actions]

🏗️ Design Decision Gate — No ADR Required

Volume threshold reached (111 new lines in pkg/), but all additions are in a test file (pkg/workflow/comment_memory_config_test.go) — no architectural decision is being made.

📋 Analysis summary

Files changed: 1 — pkg/workflow/comment_memory_config_test.go (test-only)

What this PR does: Expands test coverage for existing comment-memory config parsing logic (extractCommentMemoryConfig, parseCommentMemoryConfigValue). No production code changes; no new abstractions, patterns, or technology choices introduced.

ADR warrant check: Test coverage improvements without design trade-offs do not warrant an ADR. The pr-type:test label confirms this.

No action required. ✅

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · sonnet46 · 35.7 AIC · ⌖ 24 AIC · ⊞ 8.7K · ◷
_{Comment /review to run again}

[github-actions]

Skills-Based Review 🧠

Applied /tdd — approving with one minor coverage suggestion.

📋 Summary

Positive Highlights

• ✅ Refactored table-driven test to use full struct equality instead of a boolean flag — much stronger assertion
• ✅ TestExtractCommentMemoryConfigInvalidFields cleanly verifies the nil-return error paths
• ✅ TestParseCommentMemoryConfigValueBoolAndNil gives direct unit coverage for the three scalar branches
• ✅ Map-form case covers all documented fields (max, memory-id, target, target-repo, allowed-repos, footer)

One gap

The map-form test always supplies max and memory-id, leaving the default-fill branches (if config.Max == nil / if config.MemoryID == "") untested through that path. See inline comment for a suggested fix.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 35.4 AIC · ⌖ 7.53 AIC · ⊞ 7.1K
_{Comment /matt to run again}

Comments that could not be inline-anchored

pkg/workflow/comment_memory_config_test.go:68

[/tdd] The map-form test case always explicitly sets max and memory-id, so the default-fill paths in parseCommentMemoryConfigValue (the if config.Max == nil / if config.MemoryID == "" branches) are never exercised through the map-config route.

<details>
<summary>💡 Suggested additional test case</summary>

Add a map-form case that omits max and memory-id to verify defaults are applied:

{
    name: "map config applies defaults when max and memory-id absent",
    comment…

</details>

[github-actions]

@-

🧪 Test quality analysis by Test Quality Sentinel · haiku45 · 22.4 AIC · ⌖ 5 AIC · ⊞ 7.6K · ◷
_{Comment /review to run again}

[github-actions]

✅ Test Quality Sentinel: 100/100. All 3 new/modified test functions verify design contracts with comprehensive assertions, edge-case coverage, and no mocking violations. Test-only PR with no production code changes. Ready to merge.

[gh-aw-bot]

@copilot Quick triage nudge for this PR.

Please review the remaining feedback and finish the last test-coverage polish before maintainer handoff, then run the pr-finisher skill.

Open review context (newest first):

• Matt review summary add or confirm a map-form test case that omits max and memory-id so the default-fill branches are exercised through that path
• no failed checks were listed in the compact candidate snapshot
• branch refresh was requested if GitHub can apply it cleanly

Run: https://github.com/github/gh-aw/actions/runs/31331372172

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 13 AIC · ⌖ 5.34 AIC · ⊞ 8.5K · ◷
_{Comment /souschef to run again}