Merged
Conversation
- Добавлены события ПередИтерацией/ПослеИтерации для обработки каждой итерации бенчмарка. При наличии обработчиков итерации количество вызовов за итерацию принудительно устанавливается в 1. - Переименованы события с сохранением обратной совместимости: ПередКаждым → ПередКаждымКейсом, ПослеКаждого → ПослеКаждогоКейса
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRenamed per-case benchmark event handlers to explicit case-scoped names ( Changes
Sequence Diagram(s)sequenceDiagram
actor Client as Benchmark Client
participant Executor as Benchmark Executor
participant Events as Event System
participant Benchmark as Benchmark Code
Note over Client,Executor: High-level flow with per-case and per-iteration events
Client->>Executor: ЗапуститьКейс(Кейс)
Executor->>Events: ВызватьСобытиеПередКаждымКейсом(Кейс)
Events->>Benchmark: ПередКаждымКейсом(Контекст)
loop Warmup/Measurement/Memory
Executor->>Events: ВызватьСобытиеПередИтерацией(Этап, НомерИтерации)
Events->>Benchmark: ПередИтерацией(Контекст)
Executor->>Benchmark: ВыполнитьИтерацию()
Benchmark-->>Executor: РезультатИтерации
Executor->>Events: ВызватьСобытиеПослеИтерации(Этап, НомерИтерации, Результат)
Events->>Benchmark: ПослеИтерации(Контекст)
end
Executor->>Events: ВызватьСобытиеПослеКаждогоКейса(Кейс, РезультатКейса)
Events->>Benchmark: ПослеКаждогоКейса(Контекст)
Executor-->>Client: РезультатЗапускаКейса
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
samples/benchmarks/examples/БенчмаркСортировок.os (2)
590-658: Consider extracting shared sift-down logic to reduce duplication.
ПирамидальнаяСортировка(lines 515-586) andПирамидальнаяНаУчастке(lines 590-658) contain nearly identical sift-down logic. While inlining is noted for interpreter performance, the duplication increases maintenance burden.A helper
ПросеиваниеВниз(Массив, Узел, Граница, Смещение)could be shared. For benchmark sample code, this is optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/benchmarks/examples/БенчмаркСортировок.os` around lines 590 - 658, Both ПирамидальнаяСортировка and ПирамидальнаяНаУчастке duplicate the same sift-down loop; extract that logic into a helper ПросеиваниеВниз(Массив, Узел, Граница, Смещение) that performs the inner "пока Истина" heapify loop using Смещение instead of Низ, then replace the duplicated inner loops in both ПирамидальнаяСортировка and ПирамидальнаяНаУчастке with calls to ПросеиваниеВниз (passing Низ as Смещение in the current code), preserving all comparisons and swaps so behavior stays identical.
76-87: Sum comparison is a weak permutation check.Two distinct multisets can share the same sum (e.g.,
[1, 3]vs[2, 2]). While collision probability is low with random data, consider using a stronger validation method such as comparing sorted frequency maps or XOR-based fingerprints.💡 Alternative: XOR + sum check (still lightweight)
// Проверка 3: совпадение состава элементов (перестановка, а не подмена) СуммаИсходная = 0; СуммаРезультат = 0; + XORИсходный = 0; + XORРезультат = 0; Для Сч = 0 По Размер - 1 Цикл СуммаИсходная = СуммаИсходная + ИсходныйМассив[Сч]; СуммаРезультат = СуммаРезультат + ТестовыйМассив[Сч]; + XORИсходный = ПобитовоеИсключительноеИли(XORИсходный, ИсходныйМассив[Сч]); + XORРезультат = ПобитовоеИсключительноеИли(XORРезультат, ТестовыйМассив[Сч]); КонецЦикла; - Если СуммаИсходная <> СуммаРезультат Тогда + Если СуммаИсходная <> СуммаРезультат Или XORИсходный <> XORРезультат Тогда🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/benchmarks/examples/БенчмаркСортировок.os` around lines 76 - 87, The current permutation check using only СуммаИсходная and СуммаРезультат is weak; replace it with a stronger check by computing and comparing element-frequency maps of ИсходныйМассив and ТестовыйМассив (build associative arrays keyed by element value, increment for ИсходныйМассив, decrement for ТестовыйМассив, then ensure all counts are zero) or at minimum add an XOR fingerprint alongside the sum (compute XORИсходная/XORРезультат over elements and require both sum and XOR to match). Update the code paths that set/compare СуммаИсходная and СуммаРезультат (and where Размер is used to iterate) to instead use the chosen frequency comparison or combined sum+XOR check and throw the same ВызватьИсключение with details if the check fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/BenchmarkOneScript/core/Модули/СобытияБенчмарков.os`:
- Around line 6-16: API несовместимость: создайте экспортируемые алиасы для
старых имён событий ПередКаждым и ПослеКаждого, чтобы они ссылались на новые
константы ПередКаждымКейсом и ПослеКаждогоКейса; обновите файл, добавив перем
ПередКаждым Экспорт; Перем ПослеКаждого Экспорт; и присвоив ПередКаждым =
ПередКаждымКейсом и ПослеКаждого = ПослеКаждогоКейса (оставьте существующие
определения ПередКаждымКейсом и ПослеКаждогоКейса без изменений) чтобы старый
код продолжил работать.
---
Nitpick comments:
In `@samples/benchmarks/examples/БенчмаркСортировок.os`:
- Around line 590-658: Both ПирамидальнаяСортировка and ПирамидальнаяНаУчастке
duplicate the same sift-down loop; extract that logic into a helper
ПросеиваниеВниз(Массив, Узел, Граница, Смещение) that performs the inner "пока
Истина" heapify loop using Смещение instead of Низ, then replace the duplicated
inner loops in both ПирамидальнаяСортировка and ПирамидальнаяНаУчастке with
calls to ПросеиваниеВниз (passing Низ as Смещение in the current code),
preserving all comparisons and swaps so behavior stays identical.
- Around line 76-87: The current permutation check using only СуммаИсходная and
СуммаРезультат is weak; replace it with a stronger check by computing and
comparing element-frequency maps of ИсходныйМассив and ТестовыйМассив (build
associative arrays keyed by element value, increment for ИсходныйМассив,
decrement for ТестовыйМассив, then ensure all counts are zero) or at minimum add
an XOR fingerprint alongside the sum (compute XORИсходная/XORРезультат over
elements and require both sum and XOR to match). Update the code paths that
set/compare СуммаИсходная and СуммаРезультат (and where Размер is used to
iterate) to instead use the chosen frequency comparison or combined sum+XOR
check and throw the same ВызватьИсключение with details if the check fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 662f2b86-ef0a-4e60-8dbc-59229fbc6cbe
📒 Files selected for processing (25)
README.mddocs/БыстрыйСтарт.mddocs/ОбработчикиСобытий.mddocs/Параметризация.mdsamples/api/ДобавлениеОбработчиковСобытийПример.ossamples/benchmarks/examples/БенчмаркПоискаВКоллекциях.ossamples/benchmarks/examples/БенчмаркСборкиСтрок.ossamples/benchmarks/examples/БенчмаркСортировок.ossamples/benchmarks/examples/БенчмаркХешФункций.ossamples/benchmarks/features/БенчмаркОбработчикПередИПослеИтерации.ossamples/benchmarks/features/БенчмаркОбработчикПередИПослеКаждогоКейса.ossrc/BenchmarkOneScript/core/Классы/АннотацияПередИтерацией.ossrc/BenchmarkOneScript/core/Классы/АннотацияПередКаждымКейсом.ossrc/BenchmarkOneScript/core/Классы/АннотацияПослеИтерации.ossrc/BenchmarkOneScript/core/Классы/АннотацияПослеКаждогоКейса.ossrc/BenchmarkOneScript/core/Классы/ИзвлекательКонфигурацииБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ИсполнительБенчмарков.ossrc/BenchmarkOneScript/core/Классы/КонфигурацияБенчмарков.ossrc/BenchmarkOneScript/core/Модули/СобытияБенчмарков.ostests/fixtures/benchmarks/Классы/БенчмаркСАннотациямиКонфигурации.ostests/fixtures/benchmarks/Классы/БенчмаркСОбработчикамиСобытий.ostests/ТестыБенчмарков.ostests/ТестыКонфигурации.ostests/ТестыСериализаторНастроекЗапускаБенчмарков.ostests/ТестыЭкспортеров.os
Comment on lines
+6
to
+16
| Перем ПередКаждымКейсом Экспорт; | ||
| Перем ПослеКаждогоКейса Экспорт; | ||
| Перем ПередИтерацией Экспорт; | ||
| Перем ПослеИтерации Экспорт; | ||
|
|
||
| ПередВсеми = "ПередВсеми"; | ||
| ПослеВсех = "ПослеВсех"; | ||
| ПередКаждым = "ПередКаждым"; | ||
| ПослеКаждого = "ПослеКаждого"; | ||
| ПередВсеми = "ПередВсеми"; | ||
| ПослеВсех = "ПослеВсех"; | ||
| ПередКаждымКейсом = "ПередКаждымКейсом"; | ||
| ПослеКаждогоКейса = "ПослеКаждогоКейса"; | ||
| ПередИтерацией = "ПередИтерацией"; | ||
| ПослеИтерации = "ПослеИтерации"; |
There was a problem hiding this comment.
Сломана обратная совместимость API констант событий.
После удаления СобытияБенчмарков.ПередКаждым и СобытияБенчмарков.ПослеКаждого старый пользовательский код перестанет компилироваться/работать, хотя PR заявляет совместимость. Добавьте экспортируемые алиасы на новые значения.
💡 Предлагаемое исправление (alias для совместимости)
Перем ПередВсеми Экспорт;
Перем ПослеВсех Экспорт;
+Перем ПередКаждым Экспорт;
+Перем ПослеКаждого Экспорт;
Перем ПередКаждымКейсом Экспорт;
Перем ПослеКаждогоКейса Экспорт;
Перем ПередИтерацией Экспорт;
Перем ПослеИтерации Экспорт;
ПередВсеми = "ПередВсеми";
ПослеВсех = "ПослеВсех";
ПередКаждымКейсом = "ПередКаждымКейсом";
ПослеКаждогоКейса = "ПослеКаждогоКейса";
ПередИтерацией = "ПередИтерацией";
ПослеИтерации = "ПослеИтерации";
+ПередКаждым = ПередКаждымКейсом;
+ПослеКаждого = ПослеКаждогоКейса;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Модули/СобытияБенчмарков.os` around lines 6 - 16,
API несовместимость: создайте экспортируемые алиасы для старых имён событий
ПередКаждым и ПослеКаждого, чтобы они ссылались на новые константы
ПередКаждымКейсом и ПослеКаждогоКейса; обновите файл, добавив перем ПередКаждым
Экспорт; Перем ПослеКаждого Экспорт; и присвоив ПередКаждым = ПередКаждымКейсом
и ПослеКаждого = ПослеКаждогоКейса (оставьте существующие определения
ПередКаждымКейсом и ПослеКаждогоКейса без изменений) чтобы старый код продолжил
работать.
|
0 New Issues
0 Fixed Issues
0 Accepted Issues
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
samples/benchmarks/examples/БенчмаркСортировок.os (1)
800-808: Не скрывайте отсутствиеРазмерМассивазначением0.Такой fallback тихо превращает ошибку конфигурации в прогон по пустому массиву, и пример продолжит выдавать правдоподобный, но неверный отчёт. Для sample-кода безопаснее падать сразу.
💡 Вариант правки
Функция РазмерМассива(Кейс) - Для Каждого Параметр Из Кейс.Параметры() Цикл Если Параметр.Имя() = "РазмерМассива" Тогда Возврат Параметр.Значение(); КонецЕсли; КонецЦикла; - Возврат 0; + ВызватьИсключение "Параметр 'РазмерМассива' не найден в кейсе бенчмарка."; КонецФункции🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/benchmarks/examples/БенчмаркСортировок.os` around lines 800 - 808, Функция РазмерМассива не должна молча возвращать 0 при отсутствии параметра — это скрывает конфигурационную ошибку; вместо этого в РазмерМассива(Кейс) после перебора Кейс.Параметры() выбрасывайте/выводите явную ошибку (или вызовите Завершить/Ошибка) с понятным сообщением о том, что отсутствует параметр "РазмерМассива", используя существующие идентификаторы Параметр.Имя(), Параметр.Значение() и Кейс.Параметры() для диагностики; удалите или замените строку "Возврат 0" на логическую обработку ошибки.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@samples/benchmarks/examples/БенчмаркСортировок.md`:
- Line 30: Найдите пустой fenced code block, обозначенный тройными обратными
кавычками (```), и добавьте после них язык `text` — замените ``` на ```text,
чтобы убрать предупреждение MD040 и сделать файл lint-clean.
In `@samples/benchmarks/examples/БенчмаркСортировок.os`:
- Around line 75-86: Текущее сравнение по сумме (переменные СуммаИсходная и
СуммаРезультат) даёт ложные срабатывания; замените его на поэлементное сравнение
мультимножеств: перед проверкой создайте копию исходного массива (например
ИсходныйКопия = КопияМассива(ИсходныйМассив)), отсортируйте обе копии
(Отсортировать(ИсходныйКопия) и Отсортировать(ТестовыйКопия)) и затем в цикле
сравните элементы по индексу с использованием Размер; при несоответствии
вызовите ВызватьИсключение с информативным сообщением. Убедитесь, что вы больше
не полагаетесь на СуммаИсходная/СуммаРезультат для проверки перестановки.
---
Nitpick comments:
In `@samples/benchmarks/examples/БенчмаркСортировок.os`:
- Around line 800-808: Функция РазмерМассива не должна молча возвращать 0 при
отсутствии параметра — это скрывает конфигурационную ошибку; вместо этого в
РазмерМассива(Кейс) после перебора Кейс.Параметры() выбрасывайте/выводите явную
ошибку (или вызовите Завершить/Ошибка) с понятным сообщением о том, что
отсутствует параметр "РазмерМассива", используя существующие идентификаторы
Параметр.Имя(), Параметр.Значение() и Кейс.Параметры() для диагностики; удалите
или замените строку "Возврат 0" на логическую обработку ошибки.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c291924f-3df2-454e-8880-7b5dd0ed63df
📒 Files selected for processing (5)
samples/benchmarks/examples/БенчмаркСортировок.mdsamples/benchmarks/examples/БенчмаркСортировок.ossrc/BenchmarkOneScript/core/Модули/СобытияБенчмарков.ostests/fixtures/benchmarks/Классы/БенчмаркСОбработчикамиСобытий.ostests/fixtures/verified-report.json
✅ Files skipped from review due to trivial changes (1)
- tests/fixtures/verified-report.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/BenchmarkOneScript/core/Модули/СобытияБенчмарков.os
- tests/fixtures/benchmarks/Классы/БенчмаркСОбработчикамиСобытий.os
| | СортировкаПодсчётом | 65,000 | 100,000 | No | 99.235 ms | 1,402.77 us | 6,273.4 us | 0.62 | 0.21 | 10.077 | 50,713.4 KB | | ||
| | ПоразряднаяСортировка | 65,000 | 100,000 | No | 230.053 ms | 1,692.45 us | 7,568.9 us | 1.44 | 0.48 | 4.347 | 163,873.1 KB | | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Добавьте язык для fenced block.
markdownlint-cli2 уже ругается на MD040; здесь достаточно указать text, чтобы файл оставался lint-clean.
💡 Вариант правки
-```
+```text🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 30-30: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@samples/benchmarks/examples/БенчмаркСортировок.md` at line 30, Найдите пустой
fenced code block, обозначенный тройными обратными кавычками (```), и добавьте
после них язык `text` — замените ``` на ```text, чтобы убрать предупреждение
MD040 и сделать файл lint-clean.
Comment on lines
+75
to
+86
| // Проверка 3: совпадение состава элементов (перестановка, а не подмена) | ||
| СуммаИсходная = 0; | ||
| СуммаРезультат = 0; | ||
| Для Сч = 0 По Размер - 1 Цикл | ||
| СуммаИсходная = СуммаИсходная + ИсходныйМассив[Сч]; | ||
| СуммаРезультат = СуммаРезультат + ТестовыйМассив[Сч]; | ||
| КонецЦикла; | ||
|
|
||
| Если СуммаИсходная <> СуммаРезультат Тогда | ||
| ВызватьИсключение "Ошибка: сумма элементов изменилась (" | ||
| + СуммаРезультат + " вместо " + СуммаИсходная + ")"; | ||
| КонецЕсли; |
There was a problem hiding this comment.
Проверка перестановки здесь даёт ложноположительные результаты.
Сравнение только по сумме не гарантирует, что набор элементов сохранился: разные мультимножества могут иметь одинаковую сумму, и битая сортировка всё равно пройдёт валидацию. Надёжнее сравнить результат с отсортированной копией исходных данных поэлементно.
💡 Вариант правки
- // Проверка 3: совпадение состава элементов (перестановка, а не подмена)
- СуммаИсходная = 0;
- СуммаРезультат = 0;
- Для Сч = 0 По Размер - 1 Цикл
- СуммаИсходная = СуммаИсходная + ИсходныйМассив[Сч];
- СуммаРезультат = СуммаРезультат + ТестовыйМассив[Сч];
- КонецЦикла;
-
- Если СуммаИсходная <> СуммаРезультат Тогда
- ВызватьИсключение "Ошибка: сумма элементов изменилась ("
- + СуммаРезультат + " вместо " + СуммаИсходная + ")";
- КонецЕсли;
+ // Проверка 3: совпадение состава элементов
+ ТестоваяТЗ.Сортировать("Значение Возр");
+ Для Сч = 0 По Размер - 1 Цикл
+ Если ТестовыйМассив[Сч] <> ТестоваяТЗ[Сч].Значение Тогда
+ ВызватьИсключение "Ошибка: состав элементов изменился на позиции " + Сч;
+ КонецЕсли;
+ КонецЦикла;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@samples/benchmarks/examples/БенчмаркСортировок.os` around lines 75 - 86,
Текущее сравнение по сумме (переменные СуммаИсходная и СуммаРезультат) даёт
ложные срабатывания; замените его на поэлементное сравнение мультимножеств:
перед проверкой создайте копию исходного массива (например ИсходныйКопия =
КопияМассива(ИсходныйМассив)), отсортируйте обе копии
(Отсортировать(ИсходныйКопия) и Отсортировать(ТестовыйКопия)) и затем в цикле
сравните элементы по индексу с использованием Размер; при несоответствии
вызовите ВызватьИсключение с информативным сообщением. Убедитесь, что вы больше
не полагаетесь на СуммаИсходная/СуммаРезультат для проверки перестановки.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Добавлены события
ПередИтерацией/ПослеИтерациидля обработки каждой итерации бенчмарка. При наличии обработчиков итерации количество вызовов за итерацию принудительно устанавливается в 1.Переименованы события с сохранением обратной совместимости:
ПередКаждым→ПередКаждымКейсом,ПослеКаждого→ПослеКаждогоКейсаSummary by CodeRabbit
New Features
Documentation
Refactor
Tests