Skip to content

script readme - #472

Closed
YaroslavPerchick wants to merge 8 commits into
mainfrom
readmemarker
Closed

script readme#472
YaroslavPerchick wants to merge 8 commits into
mainfrom
readmemarker

Conversation

@YaroslavPerchick

Copy link
Copy Markdown
Collaborator

как я понял всё что нужно для оценки уже есть в README_benchmark.ipynb, скрипт вызывается ноутбуком этим, через агентов осы делает ридми со своими итерационными проверками и возвращает готовые ридми в бенчмарк для финальной оценки. по сути это облегченная версия run_multi_process.pу

если в блокноте ["OSA/.venv/Scripts/python.exe", "-m", "OSA.osa_tool.run_multi_process", ...] на ["OSA/.venv/Scripts/python.exe", "-m", "OSA.osa_tool.run_benchmark", ...] заменить локально - работает. но как конкретно в бенчмарке оно оценивается не разбирался

@andreygetmanov andreygetmanov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  1. Файл run_becnhmark без расширения .py
  2. Его нужно переместить в tests/integration
  3. Не понимаю, откуда берутся репозитории для бенчмарка. В коде упоминается таблица, но в PR её нет

@andreygetmanov
andreygetmanov requested a review from fl1pcoin June 16, 2026 13:06
@andreygetmanov

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@DRMPN

DRMPN commented Jun 19, 2026

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a45a0cf347

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@@ -0,0 +1,152 @@
import logging

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Expose the benchmark runner from the package

This runner is added only under tests/integration, while the package configuration includes only osa_tool and a repo-wide search shows no osa_tool.run_benchmark wrapper or console entry point. In environments that invoke benchmark runners as package modules, as with the existing osa_tool.run_multi_process, this module will not be importable from an installed package and the benchmark will fail before processing any repositories.

Useful? React with 👍 / 👎.

Comment thread tests/integration/run_benchmark.py Outdated
metadata=metadata,
)

dest_path = os.path.join(readmes_dir, f"{metadata.name}_README.md")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use collision-free README output paths

When the input table contains two repositories with the same project basename, for example different owners both named tool, both workers write to the same readmes/<name>_README.md path and the later copy overwrites the earlier result. That corrupts benchmark outputs while both rows can still be marked successful; include the owner/full repository name or another per-row identifier in this filename.

Useful? React with 👍 / 👎.

Comment thread tests/integration/run_benchmark.py Outdated
SourceRank(config_manager)
generate_readme(config_manager, git_agent.metadata, args)

result.update({"name": git_agent.metadata.name, "status": "Success"})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Check README generation result before succeeding

ReadmeAgent.generate_readme() catches pipeline exceptions and returns a failed result instead of raising, so an LLM, graph, or write failure still reaches this line and records Success. In that scenario the main loop skips the row on reruns and the benchmark may evaluate a missing or stale README; only mark success after verifying the generation result and destination file.

Useful? React with 👍 / 👎.

logger.error(f"Unsupported GIT platform: {repo_url}")
return result

git_agent.clone_repository()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Isolate clones for repositories with the same name

Because every worker changes into the shared repositories directory and GitAgent derives its clone directory from only the repository basename, two distinct rows such as org1/tool and org2/tool can clone into and later delete the same local path when processed in parallel. In that case one README may be generated from the wrong checkout or a worker may fail while the other is using the directory; use an owner-qualified or otherwise unique clone path for each input repository.

Useful? React with 👍 / 👎.

parser = build_parser_from_yaml(extra_sections=["settings", "arguments", "multi-run"])
args, _ = parser.parse_known_args()

args.table_path = os.path.abspath(args.table_path)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Validate table_path before normalizing it

When the runner is invoked without --table-path (the parser gives this option a None default), this abspath call raises a TypeError before load_table() can report the missing argument cleanly. That makes the benchmark fail with a traceback instead of the intended validation path; check for a missing table path before calling os.path.abspath.

Useful? React with 👍 / 👎.

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

Successfully merging this pull request may close these issues.

3 participants