Skip to content
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

Adds support for perplexity citations #34

Merged

Conversation

regismesquita
Copy link
Contributor

@regismesquita regismesquita commented Dec 11, 2024

Simple implementation, more reasonable than creating a whole new adapter
for perplexity since it's OpenAI-like. Doesn't affect other providers.
Open to changes if deemed necessary.

@regismesquita regismesquita changed the title Embed the citations on the body of the response. Support for perplexity citations Dec 11, 2024
@regismesquita regismesquita changed the title Support for perplexity citations Adds support for perplexity citations Dec 11, 2024
@Renset
Copy link
Owner

Renset commented Dec 13, 2024

@regismesquita thank you for your PR.
That's interesting, I think I also have to add Perplexity to the list of API types and test it on my own.
Regarding your changes, ideally we shouldn't embed citations into message body. I will think if some little improvements are possible and will get back to this PR later

@regismesquita
Copy link
Contributor Author

@Renset No problem, let me know if I can change this PR in any way. Having access to perplexity citations is something that I have been looking for for some time now. Most of the tools that I interact with (LiteLLM, Kerlig, Alter, and IDEs) don't offer proper support, even the ones that support perplexity, and it is such a nice feature to have :)

@Renset
Copy link
Owner

Renset commented Dec 15, 2024

@regismesquita good point about citation support, I see no reason not to implement it :)
On my side, I've added PerplexityHandler.swift to the main branch, could you please implement your changes in this file instead of ChatGPT handler and update PR? Also, I would suggest doing this bit differently (please let me know what you think): Add a URL link for each citation reference. For example, if the Perplexity API sends a response with:

<...>
This estimate is derived from detailed observations of small parts of the sky and extrapolating these observations to the whole sky[1].
<...>

and the quote array looks like this

"citations": [
    "https://www.astronomy.com/science/astro-for-kids-how-many-stars-are-there-in-space/"
]

then replace text [1] in the message with the link:

<...>
This estimate is derived from detailed observations of small parts of the sky and extrapolating these observations to the whole sky [[1]]("https://www.astronomy.com/science/astro-for-kids-how-many-stars-are-there-in-space/")
<...>

Still not the best solution, as it should be processed on the view layer IMO, but this better solution will involve a lot of changes in many files. Thus, the handler implementation as above suggests the best balance between effort and result.

@regismesquita regismesquita force-pushed the add_support_for_perplexity_citations branch from f2e1dbd to 57fc236 Compare December 16, 2024 12:39
@regismesquita
Copy link
Contributor Author

@Renset updated the handler here, It will do the search and replace strategy for non streamed responses, it will add a source line at the end with the links with the requested format and it will not generate sources when doing chat name generation.

@regismesquita
Copy link
Contributor Author

unfortunately I couldn't figure out a way of doing the search and replace method when handling streamed responses without changing the view layer to allow me to edit it.

@Renset
Copy link
Owner

Renset commented Dec 16, 2024

@regismesquita Great. I've investigated data chunks via Proxyman and see that every stream chunk starting from the very first, already contains the complete citations array, so it should be possible to make replacements inside parseDeltaJSONResponse. The example of the first chunk in response stream:

data: {"id": "9c1bf72c-b4ac-482b-XXXX-XXXXc4c35b4", "model": "llama-3.1-sonar-large-128k-online", "created": 1734286386, "usage": {"prompt_tokens": 98, "completion_tokens": 1, "total_tokens": 99}, "citations": ["https://www.astronomy.com/science/astro-for-kids-how-many-stars-are-there-in-space/", "https://www.esa.int/Science_Exploration/Space_Science/Herschel/How_many_stars_are_there_in_the_Universe", "https://bigthink.com/starts-with-a-bang/overestimated-stars-in-universe/", "https://www.youtube.com/watch?v=iujHbTvljdQ", "https://www.youtube.com/watch?v=4lbYE10Ik2I"], "object": "chat.completion", "choices": [{"index": 0, "finish_reason": null, "message": {"role": "assistant", "content": "Est"}, "delta": {"role": "assistant", "content": "Est"}}]}

@regismesquita
Copy link
Contributor Author

@Renset oh! you are correct! that made the implementation so much better. I have pushed the changes.

@Renset
Copy link
Owner

Renset commented Dec 17, 2024

@regismesquita for some reason it only works for the very first response in a chat. The other messages do not have clickable links (tested with stream responses)...
I have no idea what the cause is at the moment, but logging and debugging in Xcode should help. Take a look if you have time, otherwise I'll debug it myself in the next few days. Thanks!

CleanShot 2024-12-17 at 11 08 55@2x

@regismesquita
Copy link
Contributor Author

I did some quick runs here and noticed a similar effect, but it wasn't really just for the first one. It occurred randomly. I suspect that in the partial responses, we might not be receiving the full [1] or [2], but a partial [1, and then on the next package, the closing ], but that is only a guess.

I'm not sure if I'll be able to look into it over the next few days, but I will certainly be able to examine it over the weekend.

@regismesquita
Copy link
Contributor Author

Nevermind the above, lldb shows isGeneratingChatName = true while receiving the delta..

@regismesquita
Copy link
Contributor Author

@Renset pushed a fix. Lunch time is over, but I will take another look later. This patch seems to fix the issue; however, I'm under the impression that I have seen it happening again after this change, but I'm not sure. Will investigate a bit more later.

@regismesquita
Copy link
Contributor Author

Feel free to push any fixes if the issue is now obvious to you, Swift is not my first language.

@Renset
Copy link
Owner

Renset commented Dec 18, 2024

@regismesquita I've done a bit of testing.
I would remove the isGeneratingChatName var and checks. Generating chat name is a ~one time operation, I see no problem running formatContentWithCitations on it. Also, in my tests, Perplexity doesn't return chat names with reference links (although citations object is still persists 🤔).
If you agree, please make this change, and I'll approve & merge this PR. Thanks!

@regismesquita
Copy link
Contributor Author

@Renset done.

@Renset Renset assigned Renset and unassigned Renset Dec 18, 2024
@Renset Renset merged commit be372fe into Renset:main Dec 18, 2024
1 check passed
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