Same citation numbers #109
Conversation
| @@ -23,6 +23,9 @@ def search_authored(query: str): | |||
|
|
|||
|
|
|||
| def get_followups(query): | |||
| if not query.strip(): | |||
| return [] | |||
There was a problem hiding this comment.
this was causing errors when the server returned an error (because of the empty query) which was parsed as followups
| </A> | ||
| ); | ||
| export const CitationRef: React.FC<{citation: Citation}> = ({citation}) => { | ||
| const url = citation.url && citation.url !== "" |
There was a problem hiding this comment.
I keep wanting to add a linter, but worry that it'll mess up other PRs... :/
| setCurrent({ phase: data.phase, ...result }); | ||
| break; | ||
|
|
||
| case "citations": |
There was a problem hiding this comment.
This adds a separate step for parsing citations
| @@ -70,6 +77,32 @@ const Home: NextPage = () => { | |||
| } | |||
| }; | |||
|
|
|||
|
|
|||
| const updateCitations = (allCitations: Citation[], current: CurrentSearch) => { | |||
There was a problem hiding this comment.
The LLM flow is:
- various loading stages
- citations are passed in
- more loading phases
- the main response starts coming in in small chunks
- more loading
- the followups are fetched
- done
The main issue here is that the citations are loaded first, and there usually are like 10 possible articles to be cited, but they're not used until they appear in the actual text. And most of them won't be used anyway.
This means that magic has to be done to work out which of the possible citations are actually cited, and for them to then get the appropriate numbers.
I dislike this function. Quite a bit. But couldn't come up with a cleaner way of doing it :/
At least most of the magic is focused here, with subsequent code assuming that each citation will have an index field with the canonical number to be displayed
| citations: result?.citations || [], | ||
| base_count: result?.base_count || baseReferencesIndex, | ||
| citationsMap: findCitations(content, result?.citations || []), |
There was a problem hiding this comment.
The LLM returns citations like "bla bla bla [a] ble ble [b][c]". These then need to be mapped to the appropriate numbers. This citationMap is a mapping of {<letter>: <full citation object>}. It's possible to change from letters to numbers - it's all in the LLM prompt
mruwnik
requested review from
Aprillion,
chriscanal,
FraserLee,
ccstan99,
henri123lemoine and
Thomas-Lemoine
| setCitations(allCitations) | ||
| setCurrent(current) |
There was a problem hiding this comment.
(minor) as a first impression, it looks suspicious to me to set both of these at the same event, I would expect that all citations need to be calculated less often than whatever is "current" ... but I didn't look into it in details, I'm happy if this works as intended
There was a problem hiding this comment.
Totally agreed. This whole function is really ugly :/
This function should only be called when the current text gets updated. Which is quite a few times per query, but not that much.
current is the results of the current query, which gets updated a lot - once it's final it gets added to the history (which are previous interactions) and current is set to null (or something like that).
allCitations is the list of all displayed citations.
This function needs to update both or none - if the newest text chunk doesn't contain citations (or only known ones), then neither should be updated, but if it contains a new citation, then the map of <letter>: <citation> in the current object needs to be updated with the new indexes, but that also implies that the list of all show citations also needs to be updated with the new citations.
#42 Reuse citation numbers