DEV Community

Amir Mullagaliev
Amir Mullagaliev

Posted on

OSD 700: Sprint 3 - Code Review / First Problems

Table of Contents

  1. Introduction
  2. The Hardest Part
  3. Code Review
  4. First Review
  5. Second Review
  6. Conclusion

Introduction

During the last week or final week of sprint-2, I'd been working on follow-ups for the file attachments ui PR, there were small fixes. As per my last blog, I've written that during sprint-3 I am going to work on the reasoning tokens in ui. There is an issue:

include reasoning tokens in ui #802

deepseek returns https://api-docs.deepseek.com/guides/reasoning_model reasoning tokens. We should use html details/summary feature for this

openrouter is gonna support this for all reasoning models

this is gonna be interesting for also explicitly including reasoning context when switching models to do function calls, etc that reasoning models suck at

However, I couldn't open a pull-request, but I think that I am going to do it by the end of this week, since I've been making a research for this, and planning on asking some questions on how to integrate it.

To make a request to the model that provides deep thinking isn't complicated, but utilizing to the models that aren't supporting it isn't impossible, but hard.

As per the OpenRouter documentation there is mentioned that we may apply deep thinking into any model. In the documentation mentioned that one enthusiast has already integrated deep thinking into gpt-3.5 turbo which is impressive. Essentially, he made the model smarter:

The Hardest Part

Up till now, I've been trying to implement this new feature based on my research. However, the hardest part isn't understanding of how to use it, but how to integrate into ChatCraft.

I already have figured out which files I need to modify, and what to add. Unfortunately, I need some more time to implement it...

Code Review

At the beginning of this week, I received a notification from github that me and other student were asked to review two pull requests by professor.

It is crucial part of this course. Since we are maintainers, we must review code written by other maintainers/contributors in order to gain new skills.

First Review

First pull request was about adding useChat() hook and wrapping our application with ChatProvider, in order to have an access to the chats at any depth of web-application, so we don't have to pass chat as props to the components.

Add useChat() hook and ChatProvider context #822

Fixes #818, adding a new <ChatProvider /> context and associated useChat() hook. We can use this to access the current chat anywhere in the React tree without props.

After this lands, it would be nice to refactor our code to not bother passing the chat via props all the time.

You can test this by trying to have the LLM write SQL that references one of the files or tables in DuckDB (e.g., select * from chatcraft.chats) and then clicking the run button to execute it.

It took me a while to review the PR because I don't have an experience, but it was so much fun, for the reason, that I love to learn new things!

First thing first, I walked through all the files changed line by line to understand what'd been changed. 4 files updated:

  • src/components/CodeHeader.tsx (updated)
  • src/hooks/use-chat.tsx (new)
  • src/lib/run-code.ts (updated)
  • src/main.tsx (updated)

I knew that there is going to be something tricky since professor wanted us - his students to review it.

Eventually, I found out that in src/lib/run-code.tsx using callBack arrow function professor wasn't checking all the dependencies for the change, and missed chat value. It caused the code to always use the oldest value of chat vs. using updated version that is being triggered on change of dependency values.

My review:

Image description

Second Review

Professor decided to update the dependencies to use the newest stable versions. IMPORTANT: it doesn't mean that dependency must be the latest version, application must use STABLE version.

Update deps Feb 2025 #823

Nothing major here, should all be safe. Would appreciate people testing anything they care about and letting me know if it breaks.

To be honest, first 30 mins I didn't even know what to do, then I decided to run that branch locally. It didn't really help me because it didn't reflect any changes, so I thought: "This PR doesn't have any problems!"

Suddenly, the thought that it shouldn't be that simple entered my mind, and I decided to make a research on how to review Dependencies Change, and found this documentation that I found really helpful:

Liquid error: internal

Step by step I tested a good number of updated dependencies, walked through release notes, and found inconsistencies. Two dependencies required additional updates of DevDependencies which weren't performed, and I requested changes:

Image description

Conclusion

These three first days of the sprint weren't productive as during previous one, but I read a lot of documentation that will help me to implement deep thinking for all of the models throughout the rest of the sprint. It is going to be a huge pull request that will require tons of adjustments. I don't care about complexity, I just enjoy the process!

Top comments (0)