Skip to content

Fix/verifier contentless issues#325

Closed
stranske wants to merge 3 commits intomainfrom
fix/verifier-contentless-issues
Closed

Fix/verifier contentless issues#325
stranske wants to merge 3 commits intomainfrom
fix/verifier-contentless-issues

Conversation

@stranske
Copy link
Copy Markdown
Owner

No description provided.

The documentation incorrectly stated that pytest-xdist was missing and
tests run sequentially. In fact:

- pytest-xdist is already in pyproject.toml (version 3.8.0)
- reusable-10-ci-python.yml installs pytest-xdist automatically
- The workflow uses '-n auto --dist=loadgroup' when xdist is detected

Updated the Performance section and Recommendations to reflect the
actual implemented state.
1. Add helper functions to detect placeholder and garbage content:
   - isPlaceholderContent(): Detects "section missing from source issue" text
   - looksLikeSectionHeader(): Filters markdown headers like "## Related"
   - looksLikeReferenceLink(): Filters PR/Issue reference links

2. Update extractUncheckedItems() to use the new filters, removing:
   - Placeholder text from bot-generated PR templates
   - Markdown section headers incorrectly captured as criteria
   - PR/Issue reference links that aren't actual acceptance criteria

3. Add hasSubstantiveContent flag to formatFollowUpIssue() return:
   - true when there are real tasks/criteria/gaps to address
   - false when all content is placeholders or empty

4. Update both verifier workflows to skip issue creation when
   hasSubstantiveContent is false

Fixes issues like #313 (contentless) and #306 (garbage content).

Tested:
- PR #304 scenario: Now correctly filters out "## Related" and "- PR #N"
- PR #310 scenario: Now returns hasSubstantiveContent=false
- All 39 existing tests pass
1. Add stripCheckboxesFromScope() function that:
   - Converts checkbox items to plain bullets (scope is informational)
   - Filters out placeholder content
   - Removes empty lines

2. Update scope placeholder in issue_scope_parser.js:
   - Change from '- [ ] Scope section missing...' to '_Scope section..._'
   - Scope should use italicized text since it's not actionable

3. Update consumer repo template with same placeholder change

4. Update test to match new placeholder format

This ensures scope sections don't have checkboxes in:
- PR Automated Status Summary sections
- Verifier follow-up issues
Copilot AI review requested due to automatic review settings December 30, 2025 16: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

This PR fixes an issue where the verifier was creating follow-up issues even when they contained no substantive, actionable content. The fix includes changing the scope placeholder format from a checkbox to italic text (since scope is informational rather than actionable), adding logic to detect and skip creating issues with only placeholder content, and improving content filtering in the issue formatter.

  • Replaced checkbox-based scope placeholders with italic text format (_Scope section missing from source issue._) to reflect that scope is informational
  • Added hasSubstantiveContent check to prevent creating empty follow-up issues when all sections contain only placeholders
  • Implemented helper functions to detect and filter placeholder content, section headers, and reference links

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
.github/scripts/issue_scope_parser.js Updated scope placeholder from checkbox format to italic text format for consistency with its informational nature
templates/consumer-repo/.github/scripts/issue_scope_parser.js Applied identical scope placeholder format change to template file for consistency
.github/scripts/verifier_issue_formatter.js Added helper functions for placeholder detection, checkbox stripping, and substantive content validation; integrated hasSubstantiveContent flag into issue formatters
.github/workflows/reusable-agents-verifier.yml Added check to skip issue creation when hasSubstantiveContent is false; sets skipped output accordingly
.github/workflows/agents-verifier.yml Applied identical substantive content check and output handling as reusable workflow
.github/scripts/__tests__/issue_scope_parser.test.js Updated test expectations to match new italic text placeholder format
docs/workflows/SystemEvaluation.md Updated documentation to reflect completed parallel testing implementation

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

Comment on lines +17 to +19
return /^\s*[-*]?\s*\[[\sxX]\]\s*(scope|tasks?|acceptance\s*criteria?)\s+(section\s+)?missing\s+from\s+source\s+issue\.?\s*$/i.test(normalized) ||
/section\s+missing\s+from\s+source\s+issue/i.test(normalized) ||
/^\s*n\s*\/?\s*a\s*$/i.test(normalized) ||
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

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

The case-insensitive flag 'i' is redundant in these regex patterns since the input 'normalized' has already been converted to lowercase on line 15. Remove the 'i' flag from the regex patterns on lines 17, 18, and 19 for consistency and clarity.

Suggested change
return /^\s*[-*]?\s*\[[\sxX]\]\s*(scope|tasks?|acceptance\s*criteria?)\s+(section\s+)?missing\s+from\s+source\s+issue\.?\s*$/i.test(normalized) ||
/section\s+missing\s+from\s+source\s+issue/i.test(normalized) ||
/^\s*n\s*\/?\s*a\s*$/i.test(normalized) ||
return /^\s*[-*]?\s*\[[\sxX]\]\s*(scope|tasks?|acceptance\s*criteria?)\s+(section\s+)?missing\s+from\s+source\s+issue\.?\s*$/.test(normalized) ||
/section\s+missing\s+from\s+source\s+issue/.test(normalized) ||
/^\s*n\s*\/?\s*a\s*$/.test(normalized) ||

Copilot uses AI. Check for mistakes.
Comment on lines +14 to +89
function isPlaceholderContent(text) {
const normalized = String(text || '').toLowerCase().trim();
// Match common placeholder patterns
return /^\s*[-*]?\s*\[[\sxX]\]\s*(scope|tasks?|acceptance\s*criteria?)\s+(section\s+)?missing\s+from\s+source\s+issue\.?\s*$/i.test(normalized) ||
/section\s+missing\s+from\s+source\s+issue/i.test(normalized) ||
/^\s*n\s*\/?\s*a\s*$/i.test(normalized) ||
normalized === '';
}

/**
* Check if an item looks like a markdown section header that was incorrectly
* captured as a list item (e.g., "## Related" or "### Notes").
*
* @param {string} text - Text to check
* @returns {boolean} True if text looks like a section header
*/
function looksLikeSectionHeader(text) {
const trimmed = String(text || '').trim();
// Match markdown headers at start
return /^#{1,6}\s+\w/.test(trimmed);
}

/**
* Check if an item looks like a PR/Issue reference link rather than actual criteria.
* Matches patterns like:
* - "- PR #123 - Title"
* - "- Issue #456 - Description"
* - "PR #789 - Some fix"
*
* @param {string} text - Text to check
* @returns {boolean} True if text looks like a reference link
*/
function looksLikeReferenceLink(text) {
const trimmed = String(text || '').trim();
// Match "- PR #N" or "- Issue #N" or just "PR #N" / "Issue #N" at start
return /^[-–•]?\s*(PR|Issue|Pull\s+Request)\s*#\d+/i.test(trimmed);
}

/**
* Convert checkbox items to plain text bullets.
* Scope is informational, not something to be checked off.
* Also filters out empty/blank lines and placeholder content.
*
* @param {string} content - Content possibly containing checkboxes
* @returns {string} Content with checkboxes converted to plain bullets
*/
function stripCheckboxesFromScope(content) {
if (!content) return '';

const lines = String(content).split('\n');
const result = [];

for (const line of lines) {
const trimmed = line.trim();

// Skip empty lines
if (!trimmed) continue;

// Skip placeholder content
if (isPlaceholderContent(trimmed)) continue;

// Convert checkbox to plain bullet
const checkboxMatch = line.match(/^(\s*)[-*]\s+\[[\sxX]\]\s+(.+)$/);
if (checkboxMatch) {
const [, indent, text] = checkboxMatch;
// Skip if text is also a placeholder
if (!isPlaceholderContent(text)) {
result.push(`${indent}- ${text}`);
}
} else {
result.push(line);
}
}

return result.join('\n');
}
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

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

The newly introduced helper functions (isPlaceholderContent, looksLikeSectionHeader, looksLikeReferenceLink, and stripCheckboxesFromScope) lack test coverage. These functions are exported for testing and contain significant logic for detecting placeholders and filtering content. Add unit tests to verify their behavior with various input patterns, including edge cases like markdown formatting variations, different placeholder formats, and empty/null inputs.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 32 to 35
const PR_META_FALLBACK_PLACEHOLDERS = {
scope: '- [ ] Scope section missing from source issue.',
scope: '_Scope section missing from source issue._',
tasks: '- [ ] Tasks section missing from source issue.',
acceptance: '- [ ] Acceptance criteria section missing from source issue.',
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Placeholder detection misses scope fallback checkboxes

The PR-meta fallback placeholder for the scope section was changed to the italic _Scope section missing from source issue._, but the PR meta manager still emits the checkbox form (- [ ] Scope section missing from source issue. from .github/scripts/agents_pr_meta_update_body.js:399). Because hasNonPlaceholderScopeTasksAcceptanceContent compares section text against these constants, the checkbox version is no longer recognized as a placeholder, so a PR body containing only auto-generated placeholders now looks like “real” content. That causes the verifier skip guard to treat placeholder-only PRs/issues as having valid tasks/acceptance content and proceed, reintroducing the contentless follow-up issues this change was meant to prevent.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Closing as duplicate - these fixes are included in PR #322 which has additional enhancements.

@stranske stranske closed this Dec 30, 2025
@stranske stranske deleted the fix/verifier-contentless-issues branch December 30, 2025 18:15
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.

2 participants