-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Implement CMM-739: Generate post excerpt #24852
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
Generated by 🚫 Danger |
3f05821
to
f60e252
Compare
377fde8
to
31362a1
Compare
|
App Name | Jetpack | |
Configuration | Release-Alpha | |
Build Number | 29091 | |
Version | PR #24852 | |
Bundle ID | com.jetpack.alpha | |
Commit | 5beb232 | |
Installation URL | 2fbp30gimpao8 |
|
App Name | WordPress | |
Configuration | Release-Alpha | |
Build Number | 29091 | |
Version | PR #24852 | |
Bundle ID | org.wordpress.alpha | |
Commit | 5beb232 | |
Installation URL | 5q2mk8tjue07o |
d00970f
to
dce58de
Compare
dce58de
to
6011562
Compare
HStack(spacing: 4) { | ||
Image(systemName: "sparkle") | ||
.font(.caption2) | ||
Text(Strings.generateButton) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this "Generate" label is needed. The icon would probably be enough?
@@ -0,0 +1,128 @@ | |||
import Foundation | |||
|
|||
#if canImport(FoundationModels) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The canImport
checks can be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Claude added it for some reason and I kept it – removed.
private func startGeneration() async throws { | ||
let session = LanguageModelSession() | ||
let prompt = LanguageModelHelper.makeGenerateExcerptPrompt( | ||
content: postContent, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to use one single LanguageModelSession
, with postContent
(which won't be changed during the session) loaded into it? I think that should speed up the generation process, because it does not need to process the same postContent
again and again in the startGeneration
function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is LanguageModelSession
should be reused only if you want to keep context between interactions. I don't think this is what you want when changing the criteria. I did just add a "Suggest More Options" button to generate more results where I reuse LanguageModelSession
. From my testing, even running a simple prompt like "generate more results" in the same session still takes roughly the same amount of time as starting a new session.
excerpt-load-more.mov
case loading | ||
case error | ||
case finished | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I used it when iterating on the design of cells. I don't think we'll need it anymore – removed.
|
||
Image(systemName: "chevron.right") | ||
.font(.caption2.weight(.semibold)) | ||
.foregroundStyle(Color(.secondaryLabel).opacity(0.5)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This chevron makes me think there is a details view upon tapping it. Can it be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to show that it's tappable, but I agree I think it adds more confusion than anything else. You'll figure out that you just need to tap on one of the options. Removed.
.navigationBarTitleDisplayMode(.inline) | ||
.toolbar { | ||
ToolbarItem(placement: .topBarTrailing) { | ||
if !postContent.isEmpty && LanguageModelHelper.isSupported { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The generated result is not great when the post content is short. The result is heavily influenced by the prompt, rather than the "source content". Maybe we should only show the "Generate" button when the post content is at least a certain characters long?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened a bug https://linear.app/a8c/issue/CMM-763/if-the-post-content-is-too-short-the-results-are-not-helpful. I tried to program the model itself to do that, but it ignores this instruction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking do an if (postContent.length > 1000) { useLLM() }
check.
• Include the post's main value proposition | ||
• Use active voice (avoid "is", "are", "was", "were" when possible) | ||
• End with implicit promise of more information | ||
• Do not use ellipsis (...) at the end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The generated result is English when the content
is non-English. Adding a requirement here saying that the excerpt should be in the same language as the source content?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't going to test different locales but ran out of time.
If the post content is not supported, it will show this:

I can't get it to produce the results in anything other than English. I opened a ticket –https://linear.app/a8c/issue/CMM-762/excerpts-are-created-in-the-system-language-not-the-content-language. Help is welcome.
Excerpt 3: Lead with the primary benefit or outcome | ||
SOURCE CONTENT: | ||
\(content) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The content
is in HTML format. That does not seem to bother the LLM, because the result is pretty on point. But I wonder if we should still explicitly mention that the source content is in HTML format in the prompt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially wanted to trim HTML, but I wasn't sure it'll work well with Gutenberg blocks. Ideally, we want a rendered version of the post, but we don't have that. Based on my testing, it just figures it out, so I think we are good.
|
Fixes https://linear.app/a8c/issue/CMM-739/generate-post-excerpt
Recording
generate-excerpt.mov
Screenshots (iPhone)
Screenshots (Negative Scenarios)
Simulator.Screen.Recording.-.iPhone.17.-.2025-09-17.at.06.54.12.mov
Screenshots (iPad)