Skip to content

HelpersTask702_Describe_use_and_architecture_of_llm_transform.py_flow #703

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

gpsaggese
Copy link
Contributor

@sameer617 sameer617 added the PR for reviewers The PR needs to be reviewed by RPs label May 21, 2025
@sameer617 sameer617 marked this pull request as ready for review May 21, 2025 06:13
@sameer617 sameer617 removed the PR for reviewers The PR needs to be reviewed by RPs label May 21, 2025
@sameer617 sameer617 self-assigned this May 21, 2025
@sameer617 sameer617 added the PR for reviewers The PR needs to be reviewed by RPs label May 21, 2025
@sameer617 sameer617 requested a review from samarth9008 May 21, 2025 23:21
@sameer617
Copy link

sameer617 commented May 21, 2025

FYI @gpsaggese,

Modified Reference and How to guide of llm_transform.py

Checklist

  • The branch is named following the format
    RelatedIssueTag_Normalized_issue_title
  • Commit messages are short and informative
    • Ideally, they follow the format
      RelatedIssueTag: High-level commit description
    • E.g., HelpersTask123: Add example
    • They do not mention the name of the file that has been changed by the commit
  • The title of the PR matches the name of the branch
  • The starting post of the PR briefly describes the content of the PR on a
    high level
  • The issue related to the PR is mentioned in the starting post of the PR
  • The PR is not linked to any issues under the Development section
  • At least one reviewer is assigned under Reviewers
  • The PR author is listed under Assignees
  • All the checks performed by GitHub Actions pass
    • None of the markdown files are referenced in Readme.md
  • The branch is up to date with the master branch
  • There are no conflicts with the master branch
  • There are no files checked in by mistake (such as tmp and log files)
  • All checked in files are checked and formatted by Linter in the latest
    commit
  • No files larger than 500 KB are checked in
  • Screenshots are not used in PR posts to describe the situation or report
    an error (if needed, copy-and-paste is used instead)
  • Label PR_for_reviewers is present if a review is requested
  • Fixes addressing a review comment are applied everywhere, not just where
    the reviewer pointed out the issue
  • After addressing review comments, all corresponding conversations are
    marked as resolved
  • After all review comments are resolved, re-request review button is used
    to request another round of review

gpsaggese and others added 4 commits June 6, 2025 01:05
Pre-commit checks:
- 'check_master' passed
- 'check_author' passed
- 'check_file_size' passed
- 'check_python_compile' passed
- 'check_gitleaks' passed
All checks passed ✅
Pre-commit checks:
- 'check_master' passed
- 'check_author' passed
- 'check_file_size' passed
- 'check_python_compile' passed
- 'check_gitleaks' passed
All checks passed ✅
Pre-commit checks:
All checks passed ✅
@indrayudd indrayudd force-pushed the HelpersTask702_Describe_use_and_architecture_of_llm_transform.py_flow branch from 57ddf5e to 8835ba5 Compare June 6, 2025 05:09
@indrayudd indrayudd self-assigned this Jun 6, 2025
@indrayudd
Copy link
Collaborator

indrayudd commented Jun 6, 2025

While writing the reference, I came up with the following discrepancies and doubts:

  • review_llm doesn't work because this line file_name = hgit.find_file("all.llm_style_review_guidelines.reference.md") points to a nonexistent file
  • ⁠ code_apply_cfile ⁠ and ⁠ code_fix_from_imports ⁠ seem to be doing the same thing
  • code_fix_comments ⁠ example generates a bad comment, mentioning the variables.
  • ⁠ code_fix_logging_statements ⁠ generates a bad logging statement by not using a ⁠ _LOG ⁠ convention and using f strings
  • code_transform_apply_csfy_style ⁠ doesn’t apply all Causify rules
  • code_transform_apply_linter_instructions - I couldn’t get this working, possibly because there are some preprocessing steps and files it refers to internally for instructions, which I couldn't expose manually through the terminal
  • review_linter refers to an obsolete non-existent file. I pointed it to all.coding_style_guidelines.reference.md
  • code_transform_remove_redundancy - Adds the response to the fixed code as well.
  • difference between md_create_bullets and md_convert_text_to_bullet_points not clear, they seem to produce the same type of output
  • md_remove_formatting did not remove backticks

@gpsaggese should I create an issue for these? I'm not sure if these are bugs actively being worked on.

indrayudd and others added 4 commits June 6, 2025 04:36
…re_of_llm_transform.py_flow

Pre-commit checks:
All checks passed ✅
Pre-commit checks:
All checks passed ✅
text_rewrite
```

## Prompt Tags
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be impossible to maintain. Better to add this and let's add more comments in the code and improve the output of -p list to print the docstring

Once the LLM unit test framework is ready we will move this to the unit test which is a better way to document actively maintained code


### Container

```plantuml
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does mermaid support this natively so we can render it on GH?

In other words, can we use mermaid instead of plantuml?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried using mermaid, but the diagram just don't look good. Lines and texts overlap each other, and in my opinion will keep viewers confused.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

This is how the mermaid diagram looks BTW. I could alternatively add images directly to the markdown.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR for reviewers The PR needs to be reviewed by RPs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants