Skip to content

Commit 005509c

Browse files
lilyshen0722claudeswift
authored
fix(v2): a PR link with a card reads as a sentence (TASK-382) (#2508)
* fix(v2): a PR link with a card reads as a sentence (TASK-382) When a PR URL gets its card, the URL is removed from the text. That left '…: .' or a stray ' . ' where it had been. Replace the URL with a short owner/repo#N link instead, so the sentence keeps its shape and the card below carries the detail. Tests run through the real react-markdown. Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com> Co-authored-by: swift <WyJzd2lmdCIsImRlZmF1bHQiXQ@agents.commonly.invalid> Agent-Model: claude/claude-haiku-5-5/default * fix(v2): leave a PR URL that is already a markdown link alone (TASK-382) prUrlsToRefLinks rewrote every PR URL, including one already inside [label](url), which nested into a second link. Skip URLs preceded by (, < or [, the same guard linkifyBareUrls uses. Pinned in the real renderer suite. Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com> Co-authored-by: swift <WyJzd2lmdCIsImRlZmF1bHQiXQ@agents.commonly.invalid> Agent-Model: claude/claude-haiku-5-5/default --------- Co-authored-by: Claude Haiku 5.5 <noreply@anthropic.com> Co-authored-by: swift <WyJzd2lmdCIsImRlZmF1bHQiXQ@agents.commonly.invalid>
1 parent f8a65d5 commit 005509c

3 files changed

Lines changed: 52 additions & 3 deletions

File tree

‎frontend/src/v2/__tests__/V2MessageRow.realMarkdown.test.tsx‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// asserted here is the one a reader would click.
66
import React from 'react';
77
import { render, screen } from '@testing-library/react';
8-
import { linkifyBareUrls } from '../components/V2MessageRow';
8+
import { linkifyBareUrls, prUrlsToRefLinks } from '../components/V2MessageRow';
99
import V2GithubPrCard from '../components/V2GithubPrCard';
1010

1111
const RealReactMarkdown = jest.requireActual('../../../node_modules/react-markdown/index.js').default;
@@ -55,4 +55,40 @@ describe('bare URL linkification through the real react-markdown', () => {
5555
global.fetch = previousFetch;
5656
}
5757
});
58+
59+
// TASK-382. A PR URL that gets a card is replaced by a short owner/repo#N link,
60+
// so the sentence keeps its punctuation and words. Checked through the real renderer.
61+
// TASK-382 review: a PR URL already inside `[label](url)` must not be nested into a second link.
62+
it('a PR URL that is already a markdown link keeps its label and href', () => {
63+
const { container, unmount } = render(
64+
<RealReactMarkdown>{prUrlsToRefLinks('see [the PR](https://github.com/Team-Commonly/commonly/pull/12) now')}</RealReactMarkdown>,
65+
);
66+
const links = Array.from(container.querySelectorAll('a'));
67+
const text = container.textContent;
68+
unmount();
69+
expect(links).toHaveLength(1);
70+
expect(links[0].textContent).toBe('the PR');
71+
expect(links[0]).toHaveAttribute('href', 'https://github.com/Team-Commonly/commonly/pull/12');
72+
expect(text).toBe('see the PR now');
73+
});
74+
75+
it('a PR URL in a sentence reads as a short ref, with its punctuation intact', () => {
76+
const { container, unmount } = render(
77+
<RealReactMarkdown>{prUrlsToRefLinks('Review: https://github.com/Team-Commonly/commonly/pull/12. Then merge.')}</RealReactMarkdown>,
78+
);
79+
const text = container.textContent;
80+
const href = container.querySelector('a')?.getAttribute('href');
81+
unmount();
82+
expect(text).toBe('Review: Team-Commonly/commonly#12. Then merge.');
83+
expect(href).toBe('https://github.com/Team-Commonly/commonly/pull/12');
84+
});
85+
86+
it('a PR URL mid-sentence keeps the words around it', () => {
87+
const { container, unmount } = render(
88+
<RealReactMarkdown>{prUrlsToRefLinks('see https://github.com/Team-Commonly/commonly/pull/13 for the fix')}</RealReactMarkdown>,
89+
);
90+
const text = container.textContent;
91+
unmount();
92+
expect(text).toBe('see Team-Commonly/commonly#13 for the fix');
93+
});
5894
});

‎frontend/src/v2/__tests__/V2MessageRow.test.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ describe('V2MessageRow', () => {
6868
);
6969
expect(await screen.findByRole('link', { name: 'Pull request #374101: Card loads' })).toBeInTheDocument();
7070
const text = container.querySelector('[data-testid="react-markdown"]').textContent;
71-
expect(text).not.toContain('github.com/Team-Commonly/commonly/pull/374101');
71+
// TASK-382: the sentence keeps a short ref (anchor asserted in realMarkdown); the card is the only anchor here.
72+
expect(text).toContain('[Team-Commonly/commonly#374101](https://github.com/Team-Commonly/commonly/pull/374101)');
7273
expect(container.querySelectorAll('a[href*="pull/374101"]')).toHaveLength(1);
7374
} finally {
7475
global.fetch = previousFetch;

‎frontend/src/v2/components/V2MessageRow.tsx‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,18 @@ const MARKDOWN_IMAGE_RE = /^!\[[^\]]*\]\(([^)]+)\)$/;
201201
// so does a closing paren the URL does not open. Brackets are escaped in the link
202202
// text and percent-encoded in the destination, so a `]` or `(` in a URL cannot
203203
// close the markdown link early and leave the URL as plain text.
204+
// A PR URL that gets a card is replaced in the sentence by a short owner/repo#N
205+
// link, so the sentence keeps its words and punctuation ("Review: o/r#12.") and
206+
// the card below carries the detail. Removing the URL instead left stray
207+
// separators behind ("Review: ." and "see for it").
208+
export const prUrlsToRefLinks = (text: string): string => text.replace(
209+
// A URL already inside link syntax (`[label](url)`, `<url>`, `[url]`) is left alone.
210+
/(?<![(<[])https?:\/\/github\.com\/([\w.-]+)\/([\w.-]+)\/pull\/(\d+)(?=\b|$)/g,
211+
(_match, owner: string, repo: string, num: string) => (
212+
`[${owner}/${repo}#${num}](https://github.com/${owner}/${repo}/pull/${num})`
213+
),
214+
);
215+
204216
const LINK_DEST_ENCODE: Record<string, string> = { '(': '%28', ')': '%29', '[': '%5B', ']': '%5D', '\\': '%5C' };
205217
export const linkifyBareUrls = (text: string): string => text.replace(
206218
/(?<![(<[])https?:\/\/[^\s<>"]+/g,
@@ -540,7 +552,7 @@ const V2MessageRow: React.FC<V2MessageRowProps> = ({ message, sourceMessageId, d
540552
const prRefs = imageUrl ? [] : parseGithubPrUrls(afterFiles);
541553
let stripped = afterFiles;
542554
if (prRefs.length > 0) {
543-
stripped = afterFiles.replace(/https?:\/\/github\.com\/[\w.-]+\/[\w.-]+\/pull\/\d+(?=\b|$)/g, '').trim();
555+
stripped = prUrlsToRefLinks(afterFiles).trim();
544556
}
545557
// Auto-linkify bare http(s) URLs that aren't already inside markdown link
546558
// syntax. Without this, agents posting raw URLs render as plain text and

0 commit comments

Comments
 (0)