JOJ/JOJ3-config-generator
27
1
Fork 0
Code Issues 3 Pull Requests Actions Packages Projects Releases Wiki Activity

dev #10

Merged
李衍志523370910113 merged 238 commits from dev into master 2025-03-05 16:20:39 +08:00
Conversation 16 Commits 238 Files Changed 42 +57 -28
3 changed files with 57 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 45a184521e - Show all commits

23
joj3_config_generator/models/result.py
View File

@ -177,3 +177,26 @@ class ResultDetailConfig(BaseModel):
show_exit_status: Optional[bool] = Field(True, serialization_alias="showExitStatus")
show_runtime: Optional[bool] = Field(True, serialization_alias="showRuntime")
jon-lee marked this conversation as resolved
张泊明518370910136 commented 2025-03-05 11:40:52 +08:00
Review
Copy Link

also these Optional?

also these `Optional`?
李衍志523370910113 commented 2025-03-05 12:25:45 +08:00
Review
Copy Link

I guess some of the field have default within JOJ3, so I choose optional just to reduce the length of json previously

I guess some of the field have default within JOJ3, so I choose optional just to reduce the length of json previously
张泊明518370910136 commented 2025-03-05 12:28:00 +08:00
Review
Copy Link

better put all defaults here then we only need to check the code here

better put all defaults here then we only need to check the code here
李衍志523370910113 commented 2025-03-05 13:06:03 +08:00
Review
Copy Link

indeed.

indeed.
show_memory: Optional[bool] = Field(False, serialization_alias="showMemory")
class KeywordConfig(BaseModel):
keywords: List[str] = []
score: int = 0
class KeywordMatchConfig(BaseModel):
matches: List[KeywordConfig] = []
class FileConfig(BaseModel):
name: str = ""
class DiffCasesConfig(BaseModel):
outputs: List[DiffOutputConfig] = []
# to get minimum flexibility, may refine in future
class DiffConfig(BaseModel):
name: str = "diff"
cases: List[DiffCasesConfig] = []

32
joj3_config_generator/processers/task.py
View File

@ -1,6 +1,6 @@
import re
jon-lee marked this conversation as resolved
张泊明518370910136 commented 2025-03-03 11:03:41 +08:00
Review
Copy Link

Path should not be relative to JOJ3_CONFIG_ROOT in this file, should be relative to task.toml dir

Path should not be relative to `JOJ3_CONFIG_ROOT` in this file, should be relative to `task.toml` dir
李衍志523370910113 commented 2025-03-03 11:11:32 +08:00
Review
Copy Link

I reckon you said things is relative to JOJ3_CONFIG_ROOT in JTC before. we have a task.type in task.toml to mend the path

I reckon you said things is relative to `JOJ3_CONFIG_ROOT` in JTC before. we have a `task.type` in `task.toml` to mend the path
张泊明518370910136 commented 2025-03-03 11:15:21 +08:00
Review
Copy Link

config.path is relative to JOJ3_CONFIG_ROOT.

`config.path` is relative to `JOJ3_CONFIG_ROOT`.
李衍志523370910113 commented 2025-03-03 13:32:29 +08:00
Review
Copy Link

could you explain further? I m not quite sure my understanding is clear.

could you explain further? I m not quite sure my understanding is clear.
张泊明518370910136 commented 2025-03-04 03:05:54 +08:00
Review
Copy Link

In joj3_config_generator/models/task.py, Config.path is relative to JOJ3_CONFIG_ROOT, so task.toml will located at JOJ3_CONFIG_ROOT / task_conf.path in JTC.

In `joj3_config_generator/models/task.py`, `Config.path` is relative to `JOJ3_CONFIG_ROOT`, so `task.toml` will located at `JOJ3_CONFIG_ROOT / task_conf.path` in JTC.
import shlex
jon-lee marked this conversation as resolved
张泊明518370910136 commented 2025-03-03 11:27:04 +08:00
Review
Copy Link

Some with_.update is still using raw dict, not model with model_dump.

Some `with_.update` is still using raw dict, not model with `model_dump`.
from typing import Set
from typing import List, Set
from joj3_config_generator.models import result, task
from joj3_config_generator.models.const import JOJ3_CONFIG_ROOT
@ -76,16 +76,18 @@ def fix_keyword(
for parser in task_stage.parsers:
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-03 11:22:55 +08:00
Outdated
Review
Copy Link

do not use getattr, visit the field explictly

do not use `getattr`, visit the field explictly
李衍志523370910113 commented 2025-03-03 12:48:04 +08:00
Outdated
Review
Copy Link

resolved.

resolved.
if parser in keyword_parser:
keyword_parser_ = next(p for p in conf_stage.parsers if p.name == parser)
keyword_weight = []
keyword_weight: List[result.KeywordConfig] = []
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-05 16:06:12 +08:00
Outdated
Review
Copy Link

is there a conclusion now? or it should have a prefix TODO: ?

is there a conclusion now? or it should have a prefix `TODO: `?
李衍志523370910113 commented 2025-03-05 16:09:52 +08:00
Outdated
Review
Copy Link

so far works fine on 280 sides, indicating that pbs in 151 can be resolved with proper guidelines I think.

so far works fine on 280 sides, indicating that pbs in 151 can be resolved with proper guidelines I think.
张泊明518370910136 commented 2025-03-05 16:11:11 +08:00
Outdated
Review
Copy Link

ok, then add that prefix

ok, then add that prefix
if parser in task_stage.__dict__:
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-02 02:20:52 +08:00
Outdated
Review
Copy Link

is it in the correct unit?
Give all the fields in Limit a default value and make it non optional in task_stage.limit

is it in the correct unit? Give all the fields in `Limit` a default value and make it non optional in task_stage.limit
李衍志523370910113 commented 2025-03-02 11:47:43 +08:00
Outdated
Review
Copy Link

resolved

resolved
unique_weight = list(set(task_stage.__dict__[parser].weight))
for score in unique_weight:
keyword_weight.append({"keywords": [], "score": score})
keyword_weight.append(
result.KeywordConfig(keywords=[], score=score)
)
for idx, score in enumerate(unique_weight):
for idx_, score_ in enumerate(task_stage.__dict__[parser].weight):
if score == score_:
keyword_weight[idx]["keywords"].append(
keyword_weight[idx].keywords.append(
task_stage.__dict__[parser].keyword[idx_]
)
else:
@ -94,9 +96,9 @@ def fix_keyword(
continue
keyword_parser_.with_.update(
{
"matches": keyword_weight,
}
result.KeywordMatchConfig(
matches=keyword_weight,
).model_dump(by_alias=True)
)
return conf_stage
@ -153,7 +155,9 @@ def fix_file(task_stage: task.Stage, conf_stage: result.StageDetail) -> None:
if parser not in file_parser:
bomingzh marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-02 02:23:09 +08:00
Outdated
Review
Copy Link

these fields do not exist now

these fields do not exist now
李衍志523370910113 commented 2025-03-02 11:09:34 +08:00
Outdated
Review
Copy Link

resolved

resolved
张泊明518370910136 commented 2025-03-02 12:15:34 +08:00
Outdated
Review
Copy Link
No description provided.
continue
file_parser_ = next(p for p in conf_stage.parsers if p.name == parser)
file_parser_.with_.update({"name": task_stage.file.name})
file_parser_.with_.update(
result.FileConfig(name=task_stage.file.name).model_dump(by_alias=True)
)
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-02 02:36:32 +08:00
Outdated
Review
Copy Link

move continue to the other branch to reduce nesting

move `continue` to the other branch to reduce nesting
张泊明518370910136 commented 2025-03-02 12:15:54 +08:00
Outdated
Review
Copy Link

I mean

if parser not in keyword_parser:
    continue

if getattr(task_stage, parser, None) is None:
    continue

if score != score_:
    continue
I mean ``` if parser not in keyword_parser: continue ``` ``` if getattr(task_stage, parser, None) is None: continue ```` ``` if score != score_: continue ````
李衍志523370910113 commented 2025-03-02 17:38:50 +08:00
Outdated
Review
Copy Link

fixed.

fixed.
def fix_diff(
@ -204,8 +208,8 @@ def fix_diff(
)
if diff_output:
parser_cases.append(
{
"outputs": [
result.DiffCasesConfig(
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-02 02:28:53 +08:00
Outdated
Review
Copy Link

Create models for these dicts, then update them with the dict from model_dump

Create models for these dicts, then update them with the dict from `model_dump`
李衍志523370910113 commented 2025-03-02 11:38:27 +08:00
Outdated
Review
Copy Link

@bomingzh I don't think we can change it. This is to make proper alias so that we can get the content of result-status.

@bomingzh I don't think we can change it. This is to make proper alias so that we can get the content of `result-status`.
张泊明518370910136 commented 2025-03-02 12:09:26 +08:00
Outdated
Review
Copy Link
class DummyConfig(BaseModel):
    score: int
    comment: str

Then dummy_parser_.with_.update(dummy_config(...).model_dump()).

``` class DummyConfig(BaseModel): score: int comment: str ``` Then `dummy_parser_.with_.update(dummy_config(...).model_dump())`.
李衍志523370910113 commented 2025-03-02 17:38:42 +08:00
Outdated
Review
Copy Link

fixed

fixed
outputs=[
result.DiffOutputConfig(
score=diff_output.score,
file_name="stdout",
@ -215,13 +219,15 @@ def fix_diff(
force_quit_on_diff=diff_output.force_quit,
always_hide=diff_output.hide,
compare_space=not diff_output.ignore_spaces,
).model_dump(by_alias=True)
)
]
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-05 03:58:52 +08:00
Outdated
Review
Copy Link

When will it be None?

When will it be None?
}
)
)
if diff_parser:
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-05 04:07:34 +08:00
Outdated
Review
Copy Link

Just pass conf_stage.executor to this function rather then the whole conf_stage.

Just pass `conf_stage.executor` to this function rather then the whole `conf_stage`.
diff_parser.with_.update({"name": "diff", "cases": parser_cases})
diff_parser.with_.update(
result.DiffConfig(name="diff", cases=parser_cases).model_dump(by_alias=True)
)
conf_stage.executor.with_.cases = stage_cases
return

30
tests/convert/basic/task.json
View File

@ -222,11 +222,11 @@
}
},
"copyInCached": {
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json",
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/ex2-msan": "h7/build/ex2-msan",
"h7/build/ex2": "h7/build/ex2"
"h7/build/compile_commands.json": "h7/build/compile_commands.json"
},
"copyInDir": ".",
"copyOut": [
@ -322,11 +322,11 @@
}
},
"copyInCached": {
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json",
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/ex2-msan": "h7/build/ex2-msan",
"h7/build/ex2": "h7/build/ex2"
"h7/build/compile_commands.json": "h7/build/compile_commands.json"
},
"copyInDir": ".",
"copyOut": [
@ -444,11 +444,11 @@
"cpuSetLimit": "",
"copyIn": {},
"copyInCached": {
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json",
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/ex2-msan": "h7/build/ex2-msan",
"h7/build/ex2": "h7/build/ex2"
"h7/build/compile_commands.json": "h7/build/compile_commands.json"
},
"copyInDir": ".",
"copyOut": [
@ -542,11 +542,11 @@
"cpuSetLimit": "",
"copyIn": {},
"copyInCached": {
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json",
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/ex2-msan": "h7/build/ex2-msan",
"h7/build/ex2": "h7/build/ex2"
"h7/build/compile_commands.json": "h7/build/compile_commands.json"
},
"copyInDir": ".",
"copyOut": [
@ -639,11 +639,11 @@
"cpuSetLimit": "",
"copyIn": {},
"copyInCached": {
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json",
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/ex2-msan": "h7/build/ex2-msan",
"h7/build/ex2": "h7/build/ex2"
"h7/build/compile_commands.json": "h7/build/compile_commands.json"
},
"copyInDir": ".",
"copyOut": [