Skip to content

Add translators for Israel's national newspaper library, JTA news archive and Jerusalem Post - #3456

Closed
Execrit wants to merge 29 commits into
zotero:masterfrom
Execrit:master
Closed

Add translators for Israel's national newspaper library, JTA news archive and Jerusalem Post#3456
Execrit wants to merge 29 commits into
zotero:masterfrom
Execrit:master

Conversation

@Execrit

@Execrit Execrit commented Jun 8, 2025

Copy link
Copy Markdown

@Execrit

Execrit commented Jun 8, 2025

Copy link
Copy Markdown
Author

note: this translator can probably be applied to all veridian-ran newspaper websites with a little bit modification

@Execrit

Execrit commented Jul 17, 2025

Copy link
Copy Markdown
Author

@AbeJellinek kind reminder

@Execrit

Execrit commented Jul 17, 2025

Copy link
Copy Markdown
Author

I cannot add test cases due to cloudflare

@Execrit Execrit changed the title Add Israel's national newspaper library translator Add Israel's national newspaper library translator and JTA news archive Jul 18, 2025
@Execrit Execrit changed the title Add Israel's national newspaper library translator and JTA news archive Add translators for Israel's national newspaper library, JTA news archive and Jerusalem Post Jul 18, 2025
@Execrit

Execrit commented Aug 2, 2025

Copy link
Copy Markdown
Author

@alex-ter Could you take a look?

@alex-ter

alex-ter commented Aug 2, 2025

Copy link
Copy Markdown
Contributor

I certainly could, but just to make it clear - I'm not a maintainer here, so won't be able to merge, but hopefully the feedback will be helpful in achieving that.

I'll do a proper review later, but a few items after a quick scan. If you need help with any of the below, let me know. I could do that myself or guide you.

  1. Make sure to fix those linter errors and warnings. Looks like there are some end-of-line inconsistencies and a couple of code style things to correct.
  2. To make the CI tests run properly, you'd want to rebase on top of the current master branch (there was a CI-related fix recently).
  3. Just to make sure - it looks like at least some part of it was generated using an LLM, is that correct?
    • If so, @AbeJellinek (or maybe @dstillman? Apologies for pinging you like this folks and increasing your notification load, but this will hopefully save you time in the future as I and others will know), is there any project policy or position on such code/contributions (allowed, prohibited, don't care, etc.)? I know positions vary wildly across the FOSS community right now due to various reasons, so want to make sure I'm aligned with yours when doing these reviews.

@Execrit

Execrit commented Aug 2, 2025

Copy link
Copy Markdown
Author

I certainly could, but just to make it clear - I'm not a maintainer here

@alex-ter Oh sorry, my bad. But if you are already offering help, I have reached a conclusion that I have no idea how to fix the CRLF and LF, despite trying multiple methods. I also can't add tests to Israel's national library, perhaps you know how to do it despite the Cloudflare system. here is an example article

@Execrit Execrit closed this by deleting the head repository Aug 5, 2025
@AbeJellinek

Copy link
Copy Markdown
Member

CRLF issue: $ git config core.autocrlf true, then make some change to each of the files and stage and commit them.

LLMs: We have nothing against using LLMs for translator development. Scaffold includes code completion (currently behind a hidden pref - set scaffold.completions.mistralAPIKey to a Codestral API key). The issue with LLM-generated translator code is that people often try to generate the entire file using an LLM, and it ends up getting the format wrong, generating an invalid or mock UUID, hallucinating functions, etc. There's not a lot of Zotero translator training data out there.

Tests: Probably won't work. I'm getting a Cloudflare captcha loop in Z8. I'll see if there's anything we can do to look less suspicious to CF, but for now, we'll need to test manually in the browser.

(It looks like you closed your fork - you may need to open a new PR after undoing that.)

@Execrit

Execrit commented Aug 5, 2025

Copy link
Copy Markdown
Author

@AbeJellinek I opened a new one because this one was convoluted. See it here. I have fixed everything. I used scaffold with LLM, so everything should be fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants