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

R.pipe(R.tryCatch, ...) anomaly #3387

Closed
orsenkucher opened this issue Jun 19, 2023 · 12 comments
Closed

R.pipe(R.tryCatch, ...) anomaly #3387

orsenkucher opened this issue Jun 19, 2023 · 12 comments

Comments

@orsenkucher
Copy link

"ramda": "^0.29.0".

// deltas :: String -> [Delta]
const deltas = R.pipe(
	R.split("\n"),
	R.map(R.trim),
	R.reject(R.isEmpty),
	R.map(R.replace(/^data:/, "")),
	R.map(R.trim),
	R.map(
		R.ifElse(
			R.equals("[DONE]"),
			R.always({ isDone: true }),
			R.pipe(
				R.tap(() => {}), // NOTE: MAGIC FIX of Ramda
				R.tryCatch(JSON.parse, R.always({})),
				R.pathOr("", ["choices", 0, "delta"]),
				R.objOf("delta"),
				R.mergeRight({ isDone: false })
			)
		)
	),
	R.reject(R.isEmpty)
);

With Magic Fix I get expected results:

{ isDone: false, delta: { content: ' both' } }
{ isDone: false, delta: { content: ' the' } }
{ isDone: false, delta: { content: ' client' } }
{ isDone: false, delta: { content: ' and' } }
{ isDone: false, delta: { content: ' server' } }
{ isDone: false, delta: { content: ' side' } }
{ isDone: false, delta: { content: '.' } }
{ isDone: false, delta: {} }
{ isDone: true }

Commenting R.tap(() => {}) line what I get is:

[Function (anonymous)]
[Function (anonymous)]
[Function (anonymous)]
[Function (anonymous)]
@kedashoe
Copy link
Contributor

Hi @orsenkucher , could you post some sample input?

@orsenkucher
Copy link
Author

orsenkucher commented Jun 19, 2023

@kedashoe Hi!
Yes, sure!
I use this functions with deltas(data.toString()).
Here are few samples of console.log(JSON.stringify(data.toString()))

const data = /* Just paste string from sample */;
const res = deltas(data);
console.log(res);
"data: {\"id\":\"chatcmpl-7TFKQZlJPXyd33RDvEQKAV1QZKzE8\",\"object\":\"chat.completion.chunk\",\"created\":1687204790,\"model\":\"gpt-3.5-turbo-0613\",\"choices\":[{\"index\":0,\"delta\":{\"role\":\"assistant\",\"content\":\"\"},\"finish_reason\":null}]}\n\ndata: {\"id\":\"chatcmpl-7TFKQZlJPXyd33RDvEQKAV1QZKzE8\",\"object\":\"chat.completion.chunk\",\"created\":1687204790,\"model\":\"gpt-3.5-turbo-0613\",\"choices\":[{\"index\":0,\"delta\":{\"content\":\"Hello\"},\"finish_reason\":null}]}\n\n"
"data: {\"id\":\"chatcmpl-7TFKQZlJPXyd33RDvEQKAV1QZKzE8\",\"object\":\"chat.completion.chunk\",\"created\":1687204790,\"model\":\"gpt-3.5-turbo-0613\",\"choices\":[{\"index\":0,\"delta\":{\"content\":\"!\"},\"finish_reason\":null}]}\n\n"
"data: {\"id\":\"chatcmpl-7TFKQZlJPXyd33RDvEQKAV1QZKzE8\",\"object\":\"chat.completion.chunk\",\"created\":1687204790,\"model\":\"gpt-3.5-turbo-0613\",\"choices\":[{\"index\":0,\"delta\":{\"content\":\".js\"},\"finish_reason\":null}]}\n\n"
"data: {\"id\":\"chatcmpl-7TFKQZlJPXyd33RDvEQKAV1QZKzE8\",\"object\":\"chat.completion.chunk\",\"created\":1687204790,\"model\":\"gpt-3.5-turbo-0613\",\"choices\":[{\"index\":0,\"delta\":{\"content\":\" is\"},\"finish_reason\":null}]}\n\n"

@orsenkucher
Copy link
Author

Also if you call R.ifElse(..., ..., (x) => R.pipe(...)(x)) it will work.

@kedashoe
Copy link
Contributor

So I think the problem is the bare JSON.parse? I'm not sure why tap seems to fix this! The simplest solution is probably just adding an arrow function

      R.pipe(
        R.tryCatch(x => JSON.parse(x), R.always({})),
        R.pathOr("", ["choices", 0, "delta"]),
        R.objOf("delta"),
        R.mergeRight({ isDone: false })
      )

We created R.bind for situations like this, but it doesn't actually seem to solve it here.. not sure exactly what is going on, maybe someone else can shed some more light

@orsenkucher
Copy link
Author

orsenkucher commented Jun 19, 2023

@kedashoe Thanks for your input!

Yeah, JSON.parse seems just not to work with Ramda.
(x) => R.curry(JSON.parse)(x) breaks it again.
I'm curious too, and it'd be nice if someone else could provide further insight.

R.tap(() => {}) doesn't actually do anything, but it likely changes the context in which R.tryCatch calls JSON.parse, making the error go away. So it's better to fall to another solution.

R.bind(JSON.parse, null) isn't working as well, so using the x => JSON.parse(x) workaround is probably the best bet for now. Even though it's less elegant, at least it behaves as expected

@jlissner
Copy link

JSON.parse accepts 3 arguments, so ramda is auto-currying the remaining arguments, you can do R.unary(JSON.parse) to fix the issue:

R.pipe(
  R.tryCatch(R.unary(JSON.parse), R.always({})),
  R.pathOr("", ["choices", 0, "delta"]),
  R.objOf("delta"),
  R.mergeRight({ isDone: false })
)

@kedashoe
Copy link
Contributor

ah ty @jlissner ! That explains why adding tap first fixes things as pipe uses the first function to determine the arity of the function it creates

@orsenkucher
Copy link
Author

Thanks @jlissner!
Seems obvious now ahaha. Should I rename the issue to "Auto-currying in tryCatch with JSON.parse" or something?

@jlissner
Copy link

I don't think it would be considered an issue. Ramda has made a choice to auto curry arguments in certain settings.

You could make a post stating your case for why ramda should not auto curry in a tryCatch if you feel strongly about it.

@kedashoe
Copy link
Contributor

I don't think it would be considered an issue. Ramda has made a choice to auto curry arguments in certain settings.

Yes ramda does this pretty much everywhere to determine arity of the functions it creates, it would be quite a change to do something different at this point. It is usually the builtins/functions which accept optional arguments which catch people out. I will close now but feel free to discuss further.

@orsenkucher
Copy link
Author

Yep, I mean it's not the issue with Ramda, just was asking on how to make this github issue more indexable for other people if faced with an alike situation.

@kedashoe
Copy link
Contributor

Yep, I mean it's not the issue with Ramda, just was asking on how to make this github issue more indexable for other people if faced with an alike situation.

Ah good thought.. probably fine to leave it, but ya something like what you suggested is good

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

No branches or pull requests

3 participants