Skip to content

test: widen the issue-ref guard to all of palinode/ - #218

Merged
Paul-Kyle merged 3 commits into
phasespace-labs:mainfrom
chiruu12:fix/211-widen-issue-ref-guard
Sep 16, 2026
Merged

Paul-Kyle merged 3 commits into
phasespace-labs:mainfrom
chiruu12:fix/211-widen-issue-ref-guard

Conversation

@chiruu12

Copy link
Copy Markdown
Contributor

Closes #211.

Widens test_no_issue_refs_in_diagnostics_strings from palinode/diagnostics/ to palinode/, and clears the eight bare refs that widening exposes.

The guard was scoped to diagnostics because remediation text is printed straight at the user. That is the loudest case but not the only one. The three kinds of string it was missing:

  • two import-time assertion messages, cli/_api.py:22 and mcp.py:385, both (#377)
  • a CLI divergence warning, mcp_config.py:584, #189
  • five in the /wrap command body in cli/init.py, which palinode init renders into the user's own repo

Refs are deleted rather than swapped for a URL. Each one sat at the end of a sentence that already says what it needs to, so the number was the only part a public reader could not follow.

mcp_config.py:584 is the exception. "the silent-failure pattern documented in #189" loses its referent if the number just goes, so it now points at docs/MCP-CONFIG-HOMES.md, which is where that pattern is written up.

tests/ stays out of scope. Test strings are fixtures and assertion prose for contributors, and the pinned cases at the bottom of the guard file quote bare tags deliberately.

Checked the guard actually bites: dropped a bare #1234 into the mcp_config.py warning string and the widened test failed on it, naming the file and line. The old diagnostics-only scope never walked that file at all. Removed it again.

Full suite: 3050 passed, 1 failed. The failure is test_op_parse.py::test_parse_malformed_json_recovered_and_filtered, which fails the same way on main with this branch stashed.

Copilot AI lite review requested due to automatic review settings September 16, 2026 10:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Paul-Kyle
Paul-Kyle merged commit d0b622c into phasespace-labs:main Sep 16, 2026
13 checks passed
@Paul-Kyle

Copy link
Copy Markdown
Member

Merged — thank you, and thank you for the docstring.

Renaming the guard was the call I asked for, but writing down why tests/ stays out is the part that will still be doing work in a year. The next person to widen this will read that paragraph instead of discovering the fixture at the bottom of the file the hard way.

Two other things. Repointing mcp_config.py at docs/MCP-CONFIG-HOMES.md rather than deleting the reference is right — that sentence is the one that genuinely loses its meaning without a target, and the doc ships. And you cleared the reference in examples/hooks/palinode-session-end.sh, which neither the widened string guard nor the comment guard can see: it is outside palinode/ and it is not a .py. Fixing something the guard you just wrote cannot enforce is the harder habit.

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.

Eight private-tracker issue references ship outside palinode/diagnostics/, and the #207 guard does not reach them

3 participants