Pr for arbitrary html header injection for chainlit pai 362

[dhruvsyos]

Implemented Jinja2 for dynamic header injection for Chainlit.
Refer to issue - Chainlit#1091

[dhruvsyos]

@dokterbob As DMed, I'm unable to get Chainlit running locally on my system. I'm trying various ways to get this working but struggling a bit. Request your guidance here.

Meanwhile, I have did my best to ensure no obvious errors in the code.

[dokterbob]

The first task here is, specifically, to leave existing functionality unmodified. That is, any pre-existing configuration options should stay working as they are.

That means all the config.ui.custom_js, config.ui.custom_css etc. configuration options should function exactly they way they did before. This is a hard requirement to having our code merged into chainlit: to maintain compatibility to prevent other people's setups breaking.

Only when we implement the templating, do we create a second PR with an additional configuration option, something like config.ui.custom_headers, which, when set, will replace/disable all these other configuration options.

Note that, your PR should not be against main of our own fork but rather against the main branch of the chainlit repo, where you can engage directly with the maintainers of chainlit (I am happy to mentor you and/or provide an initial review there).

[dhruvsyos]

Only when we implement the templating, do we create a second PR with an additional configuration option, something like config.ui.custom_headers, which, when set, will replace/disable all these other configuration options.

Note that, your PR should not be against main of our own fork but rather against the main branch of the chainlit repo, where you can engage directly with the maintainers of chainlit (I am happy to mentor you and/or provide an initial review there).

Hey @dokterbob , i'm clear on all other changes. I'll start working on those asap. However, i'm not 100% clear on the above quoted part.
What i understood here is that this is next task that we may do on later stages. For that we'll create a PR on 'Chainlit' repo, engage directly with Chainlit's maintainer. Is my understanding accurate?

Also, in meanwhile, we'll continue working on this forked repo and we'll update in use this repo for our application?

[dhruvsyos]

@dokterbob
I was still struggling to get chainlit running, even via a branch by installing it using Poetry. I was getting the same error as earlier ie "frontend directory missing"
This is the relevant snippet from pyproject.toml -
chainlit = {git = "https://github.com/SynergyOS-ai/chainlit.git", subdirectory = "backend", branch = "pr-for-arbitrary-html-header-injection-for-chainlit-pai-362", develop = true}
I even tried with original Chainlit repo -
chainlit = {git = "https://github.com/Chainlit/chainlit.git", subdirectory = "backend", branch = "main", develop = true}
But still no luck, got exactly same errors.

On further investigation, I found out that there are many steps involved in setting us this package. Here's what i found from build workflow (ref - https://github.com/Chainlit/chainlit/actions/runs/9941863666/workflow) in Chainlit repo -

My understand here is that first we need to Build chainlit using pnpm run build.
Deepening my investigation, i found that this build command is atleast required for poetry to work since it making and coping the required directories -
Followings is the snippet from 'package.json'

 "buildUi": "cd libs/react-client && pnpm run build && cd ../copilot && pnpm run build && cd ../../frontend && pnpm run build",
    "build": "pnpm run buildUi && (mkdir -p backend/chainlit/frontend && cp -R frontend/dist backend/chainlit/frontend) && (mkdir -p backend/chainlit/copilot && cp -R libs/copilot/dist backend/chainlit/copilot) && (cd backend && poetry build)"

I'm looking for an approach this build the package.
Any advice from you will be appreciate here.

[dokterbob]

Only when we implement the templating, do we create a second PR with an additional configuration option, something like config.ui.custom_headers, which, when set, will replace/disable all these other configuration options.
Note that, your PR should not be against main of our own fork but rather against the main branch of the chainlit repo, where you can engage directly with the maintainers of chainlit (I am happy to mentor you and/or provide an initial review there).

Hey @dokterbob , i'm clear on all other changes. I'll start working on those asap. However, i'm not 100% clear on the above quoted part. What i understood here is that this is next task that we may do on later stages. For that we'll create a PR on 'Chainlit' repo, engage directly with Chainlit's maintainer. Is my understanding accurate?

Also, in meanwhile, we'll continue working on this forked repo and we'll update in use this repo for our application?

We'll create 2 PR's towards chainlit:

1. Using templates rather than replace() for rendering the HTML index.
2. Augmenting the template rendering with a custom parameter to override headers arbitrarily.

We're now focusing on 1.

We split them to:

1. Separate design decisions/considerations for the chainlit maintainer: better chance on having the feature merged, more overview for ourselves.
2. Reduce the steepness of the learning curve for you, so you learn better and faster and not get frustrated.

[dokterbob]

@dokterbob I was still struggling to get chainlit running, even via a branch by installing it using Poetry. I was getting the same error as earlier ie "frontend directory missing" This is the relevant snippet from pyproject.toml - chainlit = {git = "https://github.com/SynergyOS-ai/chainlit.git", subdirectory = "backend", branch = "pr-for-arbitrary-html-header-injection-for-chainlit-pai-362", develop = true} I even tried with original Chainlit repo - chainlit = {git = "https://github.com/Chainlit/chainlit.git", subdirectory = "backend", branch = "main", develop = true} But still no luck, got exactly same errors.

On further investigation, I found out that there are many steps involved in setting us this package. Here's what i found from build workflow (ref - https://github.com/Chainlit/chainlit/actions/runs/9941863666/workflow) in Chainlit repo -

My understand here is that first we need to Build chainlit using pnpm run build. Deepening my investigation, i found that this build command is atleast required for poetry to work since it making and coping the required directories - Followings is the snippet from 'package.json'

 "buildUi": "cd libs/react-client && pnpm run build && cd ../copilot && pnpm run build && cd ../../frontend && pnpm run build",
    "build": "pnpm run buildUi && (mkdir -p backend/chainlit/frontend && cp -R frontend/dist backend/chainlit/frontend) && (mkdir -p backend/chainlit/copilot && cp -R libs/copilot/dist backend/chainlit/copilot) && (cd backend && poetry build)"

I'm looking for an approach this build the package. Any advice from you will be appreciate here.

I think your analysis is correct. The 'proper' way for them to do this is to execute the NPM build as part of the Python package installation. Instead, it seems they seem to do this as part of the CI process only.

For now, doing it 'by hand' seems to be the solution.

The simplest seems to be, once you've installed the package as 'editable', to call the (p)npm commands specified. This should produce the required frontend directory.

If you experience any issues with this, I am available tomorrow to take a shot at it. Please feel free to throw me error dumps over Slack, it's where I'm the most responsive.

[dokterbob]

One more thing; you might want to change the name of index.html to index.html.j2 so our text editor's render it as a template.

[dhruvsyos]

@dokterbob I was still struggling to get chainlit running, even via a branch by installing it using Poetry. I was getting the same error as earlier ie "frontend directory missing" This is the relevant snippet from pyproject.toml - chainlit = {git = "https://github.com/SynergyOS-ai/chainlit.git", subdirectory = "backend", branch = "pr-for-arbitrary-html-header-injection-for-chainlit-pai-362", develop = true} I even tried with original Chainlit repo - chainlit = {git = "https://github.com/Chainlit/chainlit.git", subdirectory = "backend", branch = "main", develop = true} But still no luck, got exactly same errors.
On further investigation, I found out that there are many steps involved in setting us this package. Here's what i found from build workflow (ref - https://github.com/Chainlit/chainlit/actions/runs/9941863666/workflow) in Chainlit repo -
My understand here is that first we need to Build chainlit using pnpm run build. Deepening my investigation, i found that this build command is atleast required for poetry to work since it making and coping the required directories - Followings is the snippet from 'package.json'

 "buildUi": "cd libs/react-client && pnpm run build && cd ../copilot && pnpm run build && cd ../../frontend && pnpm run build",
    "build": "pnpm run buildUi && (mkdir -p backend/chainlit/frontend && cp -R frontend/dist backend/chainlit/frontend) && (mkdir -p backend/chainlit/copilot && cp -R libs/copilot/dist backend/chainlit/copilot) && (cd backend && poetry build)"

I'm looking for an approach this build the package. Any advice from you will be appreciate here.

I think your analysis is correct. The 'proper' way for them to do this is to execute the NPM build as part of the Python package installation. Instead, it seems they seem to do this as part of the CI process only.

For now, doing it 'by hand' seems to be the solution.

The simplest seems to be, once you've installed the package as 'editable', to call the (p)npm commands specified. This should produce the required frontend directory.

If you experience any issues with this, I am available tomorrow to take a shot at it. Please feel free to throw me error dumps over Slack, it's where I'm the most responsive.

I was able to solve it Mathijs. I made a simple script to do it ! It replicates the steps given under Chainlit's production workflow locally. By doing so, i'm able to build the wheel and use that in poetry for installation. This is working just fine. :-D

[dhruvsyos]

Hey @dokterbob,

We have custom headers. Made the changes instructed by you.
I have also added the Twitter Social Card.

I was not able to change the index.html to index.html.j2. I got error - "Could not resolve entry module "index.html"." I checked and update all references, but still the error persists. Please suggest if it's a critical component, and i should invest more time on this.

Kindly review. I really appreciate your patience with me on this ticket 😃🙏🏻.

• Changes on this PR
• Should i start on the second PR for " override headers arbitrarily"

[dokterbob]

Getting there!

Apart from the specific comments, it's essential that you check whether generated index.html as served from chainlit is (apart from whitespace) exactly equal as chainlit's one.

To test that: run chainlit from main, wget the index (/, I guess), then do that again from your branch and run a diff.

Also, I feel silly, I erroneously asked you to change something that was already in chainlit, which is perhaps better not to do (as it'll break backwards compatibility).

[dhruvsyos]

Getting there!

Apart from the specific comments, it's essential that you check whether generated index.html as served from chainlit is (apart from whitespace) exactly equal as chainlit's one.

To test that: run chainlit from main, wget the index (/, I guess), then do that again from your branch and run a diff.

Also, I feel silly, I erroneously asked you to change something that was already in chainlit, which is perhaps better not to do (as it'll break backwards compatibility).

Found very very few difference between index files.

I'll work on this to make them 100% same

[dhruvsyos]

Getting there!
Apart from the specific comments, it's essential that you check whether generated index.html as served from chainlit is (apart from whitespace) exactly equal as chainlit's one.
To test that: run chainlit from main, wget the index (/, I guess), then do that again from your branch and run a diff.
Also, I feel silly, I erroneously asked you to change something that was already in chainlit, which is perhaps better not to do (as it'll break backwards compatibility).

Found very very few difference between index files.

I'll work on this to make them 100% same

This is also achieved @dokterbob .

Both index files are 100% same.

[dhruvsyos]

@dokterbob In respect of your time🙏🏻, here's a brief for your review -
Here's the summary -

• i have ensured that both index files from original chainlit and forked chainlit are exactly same.
• I have removed the Twitter social card in favour of arbitrary headers(pending)

Please suggest -
If I should create this first pull request to chainlit ?
If i should start working on arbitrary headers injection ?