gopidesupavan commented on PR #63081: URL: https://github.com/apache/airflow/pull/63081#issuecomment-4027875069
> Nice work addressing all the feedback from round 1! Everything from the previous review is fixed. A few more things I noticed in this pass: > > 1. The max-iterations check runs _after_ `regenerate_with_feedback`, so the LLM call happens and then gets thrown away. If that's intentional (to preserve the last output in XCom for the UI), add a comment explaining why. > 2. `ConversationEntry.role` is typed as `str` on the Python side but `"assistant" | "human"` on the TS side. > 3. Minor: `time.sleep` before the first poll adds one unnecessary delay. > > Also, as mentioned before: please add the rationale for XCom polling vs deferral to the PR description. We discussed it on Slack but reviewers here won't have that context. yes rationale updated in the pr description with comparison and added pointers about choosing for now this approach -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
