Conversation
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. WalkthroughAdds MediaWiki redirect, import, replacement, and maintenance scripts, plus Toolforge deployment automation, scheduled jobs, and a deployment wrapper for the Python bot. ChangesMediaWiki maintenance workflows
Toolforge deployment and scheduling
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces Toolforge deployment and job configuration scripts, including deploy_repo.sh for cloning and deploying repositories, update_pybot.sh for updating the python bot, and mdwiki-jobs.yaml for scheduling various Toolforge jobs. Feedback focuses on ensuring robust execution and performance: wrapping chained commands in /bin/bash -c within the jobs configuration to prevent execution failures, adding strict bash mode (set -euo pipefail) to the update script, utilizing shallow clones (--depth 1) and more efficient find -exec syntax in the deployment script, and improving file copying and executable checks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (2)
toolforge/deploy_repo.sh (1)
122-122: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptimize file permission updates.
Using
\;with-execforks a newchmodprocess for every matching file, which can significantly slow down deployments for large repositories. Replacing\;with+will group files and pass them as arguments to a minimal number ofchmodinvocations.⚡ Proposed fix for performance optimization
- find "$TARGET_DIR" -type f ! -name "*.pyc" -exec chmod 770 {} \; + find "$TARGET_DIR" -type f ! -name "*.pyc" -exec chmod 770 {} +🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@toolforge/deploy_repo.sh` at line 122, Update the find command’s chmod invocation to use batched execution with “+” instead of per-file termination with “\;”, while preserving the existing TARGET_DIR scope, file-type filter, and *.pyc exclusion.toolforge/update_pybot.sh (1)
23-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueQuote the command invocation path to prevent word splitting.
It's a best practice to quote variables representing paths (like
$HOME) to handle cases where the path might contain spaces or special characters.🛠 Proposed fix
-# Run deploy -$HOME/shs/deploy_repo.sh "$REPO_NAME" "$TARGET_DIR" "$BRANCH" +# Run deploy +"$HOME/shs/deploy_repo.sh" "$REPO_NAME" "$TARGET_DIR" "$BRANCH"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@toolforge/update_pybot.sh` at line 23, Quote the command path in the deploy invocation within update_pybot.sh by protecting the $HOME-based path from word splitting, while preserving the existing REPO_NAME, TARGET_DIR, and BRANCH arguments.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src1/md_core/mdpy/fix_duplicate.py`:
- Around line 19-25: Update the argument-handling logic around the offset
dictionary and sys.argv loop so the parsed offset value is actually applied when
processing redirects, or remove the unused offset option entirely. Ensure
-offset:N and offset:N produce the intended limited processing behavior rather
than always processing every redirect.
- Line 14: Route all Wikipedia API access through the documented shared helpers:
in src1/md_core/mdpy/fix_duplicate.py lines 14-52, replace newapi.AllAPIS and
post_s() with the mdwiki_api.mdwiki_page or apis path; in
src1/md_core/mdpy/red.py lines 60-64, replace the raw requests.Session.post()
call with mdwiki_api.wiki_page.
In `@src1/md_core/mdpy/fixred.py`:
- Around line 105-128: Update the regex substitutions in replace_links2 to pass
re.escape(oldlink) and re.escape(oldlink2) when constructing match patterns, so
link titles with regex metacharacters are matched literally. Use a replacement
callback for the substitutions if newlink can contain backslashes, preserving
the existing replacement text and behavior.
- Around line 157-169: Preserve the original MainPage object used by the save
operation before the links loop, and use a separate variable for each link
metadata dictionary in the loop. Update the final save call to invoke save() on
the preserved MainPage object rather than the loop’s page dictionary.
In `@src1/md_core/mdpy/imp.py`:
- Around line 64-72: Update the save handling in the import flow around
page.save so a failed save does not create or save the User:Mr._Ibrahem/<title>
fallback page via MainPage. Instead, preserve the failure outcome by surfacing
or propagating the save error, leaving the intended target page unsaved without
writing unrelated user-subpage content.
In `@src1/md_core/mdpy/mdjobs.sh`:
- Around line 1-9: Add fail-fast behavior at the start of the maintenance script
so any failing command terminates the job and prevents subsequent commands from
running. Apply the existing command-path quoting consistently to every Python
invocation in the script.
In `@src1/md_core/mdpy/orred.py`:
- Around line 32-36: Update the conditional after page.create in the page
creation flow to check the returned dictionary’s success field explicitly,
logging the success message only when create["success"] is true; preserve the
existing behavior for unsuccessful creation results.
- Line 13: Update the success check around the page.create() result to inspect
create["success"] rather than evaluating the entire create response. Preserve
the existing success logging behavior only when that field indicates success, so
failed creates are not logged as successful.
In `@src1/md_core/mdpy/red.py`:
- Around line 215-283: Replace the duplicated pagination blocks in
src1/md_core/mdpy/red.py lines 215-283 and src1/md_core/mdpy/imp.py lines
168-235 with one advancing loop: handle both "all" and explicit starts through
the same Get_All_pages/work flow, evaluate termination after processing each
batch, and advance using the returned page continuation so each batch is
processed exactly once.
In `@src1/md_core/mdpy/replace1.py`:
- Around line 102-109: Validate the contents loaded by the find-file read before
processing pages, rejecting an empty or whitespace-only find string with the
existing input-error behavior. Add this guard immediately after reading find and
before loading or applying replace, while preserving non-empty find values
unchanged.
- Around line 59-79: Update the save handling around `page.save()` so
`numbers["done"] += 1` occurs only when `save_page is True`, not merely when the
result is truthy. Keep the existing revision and output-file handling aligned
with confirmed saves, ensuring failed saves do not increment the quota or
trigger completion logic.
In `@toolforge/deploy_repo.sh`:
- Line 97: Update the copy command in the deployment script to use the source
directory’s dot path instead of the * wildcard, preserving recursive copying and
the existing target while including hidden files and avoiding failures when the
source directory is empty.
---
Nitpick comments:
In `@toolforge/deploy_repo.sh`:
- Line 122: Update the find command’s chmod invocation to use batched execution
with “+” instead of per-file termination with “\;”, while preserving the
existing TARGET_DIR scope, file-type filter, and *.pyc exclusion.
In `@toolforge/update_pybot.sh`:
- Line 23: Quote the command path in the deploy invocation within
update_pybot.sh by protecting the $HOME-based path from word splitting, while
preserving the existing REPO_NAME, TARGET_DIR, and BRANCH arguments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2b152a9e-95df-45f8-a3ba-87cab844484f
📒 Files selected for processing (11)
src1/md_core/mdpy/fix_duplicate.pysrc1/md_core/mdpy/fixred.pysrc1/md_core/mdpy/imp.pysrc1/md_core/mdpy/mdjobs.shsrc1/md_core/mdpy/orred.pysrc1/md_core/mdpy/red.pysrc1/md_core/mdpy/replace1.pytoolforge/deploy_repo.shtoolforge/mdwiki-jobs.yamltoolforge/mdwiki-jobs.yaml.listtoolforge/update_pybot.sh
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 12
🧹 Nitpick comments (2)
toolforge/deploy_repo.sh (1)
122-122: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptimize file permission updates.
Using
\;with-execforks a newchmodprocess for every matching file, which can significantly slow down deployments for large repositories. Replacing\;with+will group files and pass them as arguments to a minimal number ofchmodinvocations.⚡ Proposed fix for performance optimization
- find "$TARGET_DIR" -type f ! -name "*.pyc" -exec chmod 770 {} \; + find "$TARGET_DIR" -type f ! -name "*.pyc" -exec chmod 770 {} +🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@toolforge/deploy_repo.sh` at line 122, Update the find command’s chmod invocation to use batched execution with “+” instead of per-file termination with “\;”, while preserving the existing TARGET_DIR scope, file-type filter, and *.pyc exclusion.toolforge/update_pybot.sh (1)
23-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueQuote the command invocation path to prevent word splitting.
It's a best practice to quote variables representing paths (like
$HOME) to handle cases where the path might contain spaces or special characters.🛠 Proposed fix
-# Run deploy -$HOME/shs/deploy_repo.sh "$REPO_NAME" "$TARGET_DIR" "$BRANCH" +# Run deploy +"$HOME/shs/deploy_repo.sh" "$REPO_NAME" "$TARGET_DIR" "$BRANCH"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@toolforge/update_pybot.sh` at line 23, Quote the command path in the deploy invocation within update_pybot.sh by protecting the $HOME-based path from word splitting, while preserving the existing REPO_NAME, TARGET_DIR, and BRANCH arguments.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src1/md_core/mdpy/fix_duplicate.py`:
- Around line 19-25: Update the argument-handling logic around the offset
dictionary and sys.argv loop so the parsed offset value is actually applied when
processing redirects, or remove the unused offset option entirely. Ensure
-offset:N and offset:N produce the intended limited processing behavior rather
than always processing every redirect.
- Line 14: Route all Wikipedia API access through the documented shared helpers:
in src1/md_core/mdpy/fix_duplicate.py lines 14-52, replace newapi.AllAPIS and
post_s() with the mdwiki_api.mdwiki_page or apis path; in
src1/md_core/mdpy/red.py lines 60-64, replace the raw requests.Session.post()
call with mdwiki_api.wiki_page.
In `@src1/md_core/mdpy/fixred.py`:
- Around line 105-128: Update the regex substitutions in replace_links2 to pass
re.escape(oldlink) and re.escape(oldlink2) when constructing match patterns, so
link titles with regex metacharacters are matched literally. Use a replacement
callback for the substitutions if newlink can contain backslashes, preserving
the existing replacement text and behavior.
- Around line 157-169: Preserve the original MainPage object used by the save
operation before the links loop, and use a separate variable for each link
metadata dictionary in the loop. Update the final save call to invoke save() on
the preserved MainPage object rather than the loop’s page dictionary.
In `@src1/md_core/mdpy/imp.py`:
- Around line 64-72: Update the save handling in the import flow around
page.save so a failed save does not create or save the User:Mr._Ibrahem/<title>
fallback page via MainPage. Instead, preserve the failure outcome by surfacing
or propagating the save error, leaving the intended target page unsaved without
writing unrelated user-subpage content.
In `@src1/md_core/mdpy/mdjobs.sh`:
- Around line 1-9: Add fail-fast behavior at the start of the maintenance script
so any failing command terminates the job and prevents subsequent commands from
running. Apply the existing command-path quoting consistently to every Python
invocation in the script.
In `@src1/md_core/mdpy/orred.py`:
- Around line 32-36: Update the conditional after page.create in the page
creation flow to check the returned dictionary’s success field explicitly,
logging the success message only when create["success"] is true; preserve the
existing behavior for unsuccessful creation results.
- Line 13: Update the success check around the page.create() result to inspect
create["success"] rather than evaluating the entire create response. Preserve
the existing success logging behavior only when that field indicates success, so
failed creates are not logged as successful.
In `@src1/md_core/mdpy/red.py`:
- Around line 215-283: Replace the duplicated pagination blocks in
src1/md_core/mdpy/red.py lines 215-283 and src1/md_core/mdpy/imp.py lines
168-235 with one advancing loop: handle both "all" and explicit starts through
the same Get_All_pages/work flow, evaluate termination after processing each
batch, and advance using the returned page continuation so each batch is
processed exactly once.
In `@src1/md_core/mdpy/replace1.py`:
- Around line 102-109: Validate the contents loaded by the find-file read before
processing pages, rejecting an empty or whitespace-only find string with the
existing input-error behavior. Add this guard immediately after reading find and
before loading or applying replace, while preserving non-empty find values
unchanged.
- Around line 59-79: Update the save handling around `page.save()` so
`numbers["done"] += 1` occurs only when `save_page is True`, not merely when the
result is truthy. Keep the existing revision and output-file handling aligned
with confirmed saves, ensuring failed saves do not increment the quota or
trigger completion logic.
In `@toolforge/deploy_repo.sh`:
- Line 97: Update the copy command in the deployment script to use the source
directory’s dot path instead of the * wildcard, preserving recursive copying and
the existing target while including hidden files and avoiding failures when the
source directory is empty.
---
Nitpick comments:
In `@toolforge/deploy_repo.sh`:
- Line 122: Update the find command’s chmod invocation to use batched execution
with “+” instead of per-file termination with “\;”, while preserving the
existing TARGET_DIR scope, file-type filter, and *.pyc exclusion.
In `@toolforge/update_pybot.sh`:
- Line 23: Quote the command path in the deploy invocation within
update_pybot.sh by protecting the $HOME-based path from word splitting, while
preserving the existing REPO_NAME, TARGET_DIR, and BRANCH arguments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2b152a9e-95df-45f8-a3ba-87cab844484f
📒 Files selected for processing (11)
src1/md_core/mdpy/fix_duplicate.pysrc1/md_core/mdpy/fixred.pysrc1/md_core/mdpy/imp.pysrc1/md_core/mdpy/mdjobs.shsrc1/md_core/mdpy/orred.pysrc1/md_core/mdpy/red.pysrc1/md_core/mdpy/replace1.pytoolforge/deploy_repo.shtoolforge/mdwiki-jobs.yamltoolforge/mdwiki-jobs.yaml.listtoolforge/update_pybot.sh
🛑 Comments failed to post (12)
src1/md_core/mdpy/fix_duplicate.py (2)
14-14: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== files ==\n' git ls-files 'src1/md_core/mdpy/*' | sed -n '1,120p' printf '\n== outlines ==\n' for f in src1/md_core/mdpy/fix_duplicate.py src1/md_core/mdpy/red.py; do echo "--- $f" ast-grep outline "$f" --view expanded || true done printf '\n== helper search ==\n' rg -n "mdwiki_api\.wiki_page|mdwiki_api\.mdwiki_page|from .* import apis|def .*apis|class .*apis|newapi\.AllAPIS|Session\.post\(" src1 -SRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 2356
🏁 Script executed:
#!/bin/bash set -euo pipefail for f in src1/md_core/mdpy/fix_duplicate.py src1/md_core/mdpy/red.py; do echo "===== $f (lines) =====" wc -l "$f" done echo "===== fix_duplicate.py =====" sed -n '1,140p' src1/md_core/mdpy/fix_duplicate.py echo "===== red.py =====" sed -n '1,170p' src1/md_core/mdpy/red.py echo "===== helper definitions =====" for f in $(git ls-files 'src1/**/mdwiki_api/*.py' 'src1/**/mdwiki_api/**.py' 2>/dev/null | head -n 20); do echo "--- $f" done rg -n "class NewApi|def .*post|class MainPage|requests.Session|Session\.post|mdwiki_page" src1 -SRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 10047
🏁 Script executed:
#!/bin/bash set -euo pipefail for f in src1/md_core/mdpy/fix_duplicate.py src1/md_core/mdpy/red.py; do echo "===== $f (lines) =====" wc -l "$f" done echo "===== fix_duplicate.py =====" sed -n '1,140p' src1/md_core/mdpy/fix_duplicate.py echo "===== red.py =====" sed -n '1,170p' src1/md_core/mdpy/red.py echo "===== helper definitions =====" rg -n "class NewApi|def .*post|class MainPage|requests.Session|Session\.post|mdwiki_page" src1 -SRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 10047
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "===== newapi references =====" rg -n "\bnewapi\b|from newapi import|import newapi" src1 -S echo "===== mdwiki_api files =====" git ls-files 'src1/**/mdwiki_api/*.py' | sed -n '1,120p' echo "===== wiki_page / mdwiki_page symbols =====" rg -n "def |class " src1/**/mdwiki_api/*.py -S || trueRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 2143
Route MediaWiki API access through the shared helpers.
src1/md_core/mdpy/fix_duplicate.py#L14-L52: replacenewapi.AllAPIS/post_s()with the documentedmdwiki_api.mdwiki_pageorapispath.src1/md_core/mdpy/red.py#L38-L64: replace the rawrequests.Session.post()call to the Wikipedia API withmdwiki_api.wiki_page.📍 Affects 2 files
src1/md_core/mdpy/fix_duplicate.py#L14-L14(this comment)src1/md_core/mdpy/red.py#L60-L64🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src1/md_core/mdpy/fix_duplicate.py` at line 14, Route all Wikipedia API access through the documented shared helpers: in src1/md_core/mdpy/fix_duplicate.py lines 14-52, replace newapi.AllAPIS and post_s() with the mdwiki_api.mdwiki_page or apis path; in src1/md_core/mdpy/red.py lines 60-64, replace the raw requests.Session.post() call with mdwiki_api.wiki_page.Source: Coding guidelines
19-25: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Apply or remove the
offsetoption.The parsed value is never read, so
-offset:Nstill processes every redirect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src1/md_core/mdpy/fix_duplicate.py` around lines 19 - 25, Update the argument-handling logic around the offset dictionary and sys.argv loop so the parsed offset value is actually applied when processing redirects, or remove the unused offset option entirely. Ensure -offset:N and offset:N produce the intended limited processing behavior rather than always processing every redirect.src1/md_core/mdpy/fixred.py (2)
105-128: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Map the file structure first, then inspect the relevant section. ast-grep outline src1/md_core/mdpy/fixred.py --view expanded || true echo "---- imports and symbols ----" sed -n '1,220p' src1/md_core/mdpy/fixred.py | nl -ba | sed -n '1,220p'Repository: Mdwiki-TD/mdwiki-python-files
Length of output: 575
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- src1/md_core/mdpy/fixred.py lines 1-220 ----" sed -n '1,220p' src1/md_core/mdpy/fixred.py echo "---- grep for replace_links2 callers ----" rg -n "replace_links2\(" -S src1/md_core/mdpy/fixred.py src1 || true echo "---- grep for oldlink normalization sources ----" rg -n "normalized|from_to|find_redirects|load_nonredirects" -S src1/md_core/mdpy/fixred.pyRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 8579
Escape link titles before building the regex. Titles with regex metacharacters (for example
C++orFoo(Bar)) can makere.subfail or replace the wrong text. Usere.escape(oldlink)/re.escape(oldlink2), and switch to a replacement callback ifnewlinkmay contain backslashes.🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 109-109: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: text.find(f"[[{oldlink}]]")
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').(xpath-injection-python)
[warning] 110-110: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: text.find(f"[[{oldlink}|")
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').(xpath-injection-python)
[warning] 111-111: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: text.find(f"[[{oldlink2}]]")
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').(xpath-injection-python)
[warning] 112-112: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: text.find(f"[[{oldlink2}|")
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').(xpath-injection-python)
[warning] 120-120: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.sub(r"[[%s(|]])" % oldlink, r"[[%s\g<1>" % newlink, text, flags=re.IGNORECASE)
Note: [CWE-1333] Inefficient Regular Expression Complexity.(redos-non-literal-regex-python)
[warning] 123-123: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.sub(r"[[%s(|]])" % oldlink2, r"[[%s\g<1>" % newlink, text, flags=re.IGNORECASE)
Note: [CWE-1333] Inefficient Regular Expression Complexity.(redos-non-literal-regex-python)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src1/md_core/mdpy/fixred.py` around lines 105 - 128, Update the regex substitutions in replace_links2 to pass re.escape(oldlink) and re.escape(oldlink2) when constructing match patterns, so link titles with regex metacharacters are matched literally. Use a replacement callback for the substitutions if newlink can contain backslashes, preserving the existing replacement text and behavior.Source: Linters/SAST tools
157-169: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Preserve the
MainPageobject used for saving.Line 159 replaces
pagewith a link metadata dictionary, so Line 169 callssave()on adictwhenever the page has links.Proposed fix
- page = md_MainPage(title, "www", family="mdwiki") - exists = page.exists() - text = page.get_text() + wiki_page = md_MainPage(title, "www", family="mdwiki") + exists = wiki_page.exists() + text = wiki_page.get_text() ... - page = links["links"][tt] - tit = page["title"] - tit2 = normalized.get(page["title"], page["title"]) + link_data = links["links"][tt] + tit = link_data["title"] + tit2 = normalized.get(tit, tit) ... - save_page = page.save(newtext=newtext, summary="Fix redirects") + save_page = wiki_page.save(newtext=newtext, summary="Fix redirects")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.for tt in links["links"]: # --- link_data = links["links"][tt] tit = link_data["title"] tit2 = normalized.get(tit, tit) # --- if fixed_tit := from_to.get(tit) or from_to.get(tit2): newtext = replace_links2(newtext, tit, fixed_tit) elif tit not in nonredirects: if tit2 != tit: logger.info(f'<<red>> tit:["{tit}"] and tit:["{tit2}"] not in from_to') # --- save_page = wiki_page.save(newtext=newtext, summary="Fix redirects")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src1/md_core/mdpy/fixred.py` around lines 157 - 169, Preserve the original MainPage object used by the save operation before the links loop, and use a separate variable for each link metadata dictionary in the loop. Update the final save call to invoke save() on the preserved MainPage object rather than the loop’s page dictionary.src1/md_core/mdpy/imp.py (1)
64-72: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== outline: src1/md_core/mdpy/imp.py ==\n' ast-grep outline src1/md_core/mdpy/imp.py --view expanded || true printf '\n== relevant slice: src1/md_core/mdpy/imp.py ==\n' sed -n '1,140p' src1/md_core/mdpy/imp.py | cat -n printf '\n== search for related save fallback patterns ==\n' rg -n "User:Mr\._Ibrahem|Returns the article text after importing the history|page2\.save|save_page is not True|MainPage\(" src1 -SRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 7044
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path p = Path('src1/md_core/mdpy/imp.py') print(p.exists(), p.stat().st_size) for i, line in enumerate(p.read_text().splitlines(), 1): if 50 <= i <= 95: print(f"{i:4d}: {line}") PYRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 1793
Don't save failed imports to a user subpage. If
page.save()fails, this fallback writes the imported article text toUser:Mr._Ibrahem/<title>instead of retrying or surfacing the error, leaving the target page unsaved and creating unrelated wiki content.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src1/md_core/mdpy/imp.py` around lines 64 - 72, Update the save handling in the import flow around page.save so a failed save does not create or save the User:Mr._Ibrahem/<title> fallback page via MainPage. Instead, preserve the failure outcome by surfacing or propagating the save error, leaving the intended target page unsaved without writing unrelated user-subpage content.src1/md_core/mdpy/mdjobs.sh (1)
1-9: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Fail the maintenance job when any command fails.
Without
set -e, an earlier failure is masked by the final command’s exit status and subsequent maintenance continues.Proposed fix
#!/bin/bash +set -euo pipefail -$HOME/local/bin/python3 core8/pwb.py md_core/mdpy/fix_duplicate save +"$HOME/local/bin/python3" core8/pwb.py md_core/mdpy/fix_duplicate saveApply the quoting consistently to the remaining commands.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 3-3: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 5-5: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 7-7: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 9-9: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src1/md_core/mdpy/mdjobs.sh` around lines 1 - 9, Add fail-fast behavior at the start of the maintenance script so any failing command terminates the job and prevents subsequent commands from running. Apply the existing command-path quoting consistently to every Python invocation in the script.src1/md_core/mdpy/orred.py (2)
13-13: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== repo files of interest ==\n' git ls-files 'src1/md_core/mdpy/orred.py' 'src1/**/MainPage*' 'src1/**/mdwiki_sql*' 'src1/**/sql_for_mdwiki*' 2>/dev/null || true printf '\n== outline: src1/md_core/mdpy/orred.py ==\n' ast-grep outline src1/md_core/mdpy/orred.py --view expanded || true printf '\n== file: src1/md_core/mdpy/orred.py ==\n' cat -n src1/md_core/mdpy/orred.py printf '\n== search for sql_for_mdwiki and mdwiki_sql ==\n' rg -n "sql_for_mdwiki|mdwiki_sql|select_md_sql" src1 || true printf '\n== search for MainPage.create definitions/usages ==\n' rg -n "class MainPage|def create\(" src1 || trueRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 4606
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== inspect nearby code for MainPage.create call site ==\n' rg -n -A6 -B6 "MainPage\.create|create\(.*\)" src1/md_core/mdpy/orred.py src1 || trueRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 3915
🌐 Web query:
mdwiki_api MainPage.create return value dictionary truthy create summary result💡 Result:
There is no standard or widely recognized library named mdwiki that contains a method called MainPage.create. MDwiki is predominantly known as a static, client-side Markdown wiki framework (originally by Dynalon) that runs entirely in the browser using HTML5 and JavaScript, requiring no server-side processing or API calls to create pages [1][2][3]. In this context, pages are created by simply adding new.md files to the file system [3][4]. Several other projects use the name mdwiki (or similar variations), such as a Python-based Flask implementation [5][6] and a Go-based project [7][8], but none of these documented repositories expose a library method named MainPage.create. If you are encountering this code, it is likely part of a specific, custom, or internal implementation rather than a public-facing API. Based on standard Python dictionary behavior, if a function returns a dictionary, it is considered truthy if it is non-empty (i.e., contains at least one key-value pair) [5].
Citations:
- 1: http://www.mdwiki.info/
- 2: https://github.com/dynalon/mdwiki
- 3: https://www.duskopijetlovic.com/howto/git/versioncontrol/sysadmin/cli/notebook/diary/journal/plaintext/text/terminal/shell/knowledge/projectmanagement/unix/graph/diagram/visualization/wiki/technicalwriting/documentation/writing/tex/latex/pdf/markdown/web/webbrowser/webdevelopment/software/coding/programming/opensource/reference/2024/02/04/git-notes.html
- 4: https://github.com/exalted/mdwiki-seed
- 5: https://github.com/xbynet/mdwiki
- 6: https://gitee.com/mirrors/Python-mdwiki
- 7: https://github.com/rostislavjadavan/mdwiki
- 8: https://pkg.go.dev/github.com/rostislavjadavan/mdwiki
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== files under db/mdapi_sql ==\n' git ls-files 'db/mdapi_sql/**' 'src1/**/db/mdapi_sql/**' 2>/dev/null || true printf '\n== search for select_md_sql definition and wrappers ==\n' rg -n -A4 -B4 "def select_md_sql|mdwiki_sql|get_db_categories" . || true printf '\n== search for mdapi_sql import styles ==\n' rg -n "from db\.mdapi_sql import sql_for_mdwiki|from db\.mdapi_sql\.services import sql_for_mdwiki|from mdapi_sql import sql_for_mdwiki" . || trueRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 50387
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== inspect README/docs for mdwiki_sql references ==\n' rg -n "mdwiki_sql|select_md_sql|sql_for_mdwiki" README* . || true printf '\n== locate MainPage class reference in repo ==\n' rg -n "class MainPage|MainPage\(" . || trueRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 32298
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== outline: src/newapi/client_wiki/pages/super_page.py ==\n' ast-grep outline src/newapi/client_wiki/pages/super_page.py --view expanded || true printf '\n== relevant slices: src/newapi/client_wiki/pages/super_page.py ==\n' sed -n '1,220p' src/newapi/client_wiki/pages/super_page.py printf '\n== mdwiki wrapper: src/mdwiki_api/wiki_page.py ==\n' sed -n '1,120p' src/mdwiki_api/wiki_page.pyRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 10647
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== create() implementation in src/newapi/client_wiki/pages/super_page.py ==\n' sed -n '799,874p' src/newapi/client_wiki/pages/super_page.py printf '\n== any direct type hints/docs for create() return ==\n' rg -n -A3 -B3 "def create\(" src/newapi/client_wiki/pages/super_page.pyRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 2876
Check
create["success"]instead of the whole response here.page.create()returns a non-empty dict on both success and failure, soif create:logs failed creates as successful.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src1/md_core/mdpy/orred.py` at line 13, Update the success check around the page.create() result to inspect create["success"] rather than evaluating the entire create response. Preserve the existing success logging behavior only when that field indicates success, so failed creates are not logged as successful.Source: Coding guidelines
32-36: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate the file and inspect the relevant section. git ls-files | rg '^src1/md_core/mdpy/orred\.py$' || true wc -l src1/md_core/mdpy/orred.py sed -n '1,120p' src1/md_core/mdpy/orred.py # Find where page.create is used or defined. rg -n "page\.create\(|def create\(" src1 -g '!**/__pycache__/**'Repository: Mdwiki-TD/mdwiki-python-files
Length of output: 3064
🏁 Script executed:
#!/bin/bash set -euo pipefail # Search for page.create usage patterns and any handling of its return value. rg -n "create\s*=\s*page\.create\(|if\s+page\.create\(|\.get\(\"success\"\)|success\]" src1 -g '!**/__pycache__/**' # Inspect nearby code in files that use page.create to infer expected return type. sed -n '220,290p' src1/copy_to_en/medwiki.py sed -n '170,225p' src1/copy_to_en/mdwikicx.py sed -n '1,120p' src1/copy_to_en/bot.pyRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 5494
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate the implementation or documentation for mdwiki_api.wiki_page.MainPage.create. fd -a "wiki_page.py" . rg -n "class MainPage|def create\(" . -g '!*__pycache__*' -g '!*.pyc' # If the module lives in the repo, inspect the relevant definition area. for f in $(fd -a "wiki_page.py" .); do echo "=== $f ===" wc -l "$f" sed -n '1,260p' "$f" doneRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 3009
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the MainPage.create implementation. sed -n '760,860p' src/newapi/client_wiki/pages/super_page.py # Also inspect any references to a returned "success" field in the same module. rg -n '"success"|success\)' src/newapi/client_wiki/pages/super_page.pyRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 4060
🏁 Script executed:
#!/bin/bash set -euo pipefail # Read the create() return path around the success/failure branches. sed -n '700,760p' src/newapi/client_wiki/pages/super_page.pyRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 1995
Check
create["success"]before logging success.
page.create(...)returns a dict, soif create:also treats{"success": False, ...}as truthy and logs a successful creation on failures.Suggested change
- if create: + if create.get("success") is True: logger.info(f"<<green>>** true .. [[or:{mdtitle}]] ")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if not page.exists(): create = page.create(text=text, summary=sus) # --- if create.get("success") is True: logger.info(f"<<green>>** true .. [[or:{mdtitle}]] ")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src1/md_core/mdpy/orred.py` around lines 32 - 36, Update the conditional after page.create in the page creation flow to check the returned dictionary’s success field explicitly, logging the success message only when create["success"] is true; preserve the existing behavior for unsuccessful creation results.src1/md_core/mdpy/red.py (1)
215-283: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Consolidate the copied pagination into one advancing loop.
Both implementations process batches before entering a second pagination block and mutate
okaybefore progress is evaluated, causing duplicate batches or incomplete"all"runs.
src1/md_core/mdpy/red.py#L215-L283: remove both pagination blocks and process each continuation exactly once.src1/md_core/mdpy/imp.py#L168-L235: apply the same single-loop pagination structure.🧰 Tools
🪛 Ruff (0.15.21)
[error] 227-227: Variable
listis shadowing a Python builtin(A001)
[error] 240-240: Variable
listis shadowing a Python builtin(A001)
[error] 243-243: Variable
listis shadowing a Python builtin(A001)
[error] 245-245: Variable
listis shadowing a Python builtin(A001)
[error] 247-247: Variable
listis shadowing a Python builtin(A001)
[error] 262-262: Variable
listis shadowing a Python builtin(A001)
[error] 278-278: Variable
listis shadowing a Python builtin(A001)
📍 Affects 2 files
src1/md_core/mdpy/red.py#L215-L283(this comment)src1/md_core/mdpy/imp.py#L168-L235🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src1/md_core/mdpy/red.py` around lines 215 - 283, Replace the duplicated pagination blocks in src1/md_core/mdpy/red.py lines 215-283 and src1/md_core/mdpy/imp.py lines 168-235 with one advancing loop: handle both "all" and explicit starts through the same Get_All_pages/work flow, evaluate termination after processing each batch, and advance using the returned page continuation so each batch is processed exactly once.src1/md_core/mdpy/replace1.py (2)
59-79: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Map the target file and inspect the relevant region. ast-grep outline src1/md_core/mdpy/replace1.py --view expanded || true echo "----" nl -ba src1/md_core/mdpy/replace1.py | sed -n '1,140p' echo "---- save() call sites ----" rg -n "\.save\(" src1/md_core/mdpy -g '*.py' || true echo "---- MainPage definitions/usages ----" rg -n "class MainPage|def save\(" src1 -g '*.py' || trueRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 602
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- replace1.py (relevant region) ----" sed -n '1,140p' src1/md_core/mdpy/replace1.py echo "---- search for MainPage.save definitions/usages ----" rg -n "def save\(" src1 -g '*.py' || true rg -n "class MainPage|class .*Page" src1 -g '*.py' || true echo "---- search for NewApi and page.save callers ----" rg -n "NewApi\(|page\.save\(" src1 -g '*.py' || trueRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 6284
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- search for mdwiki_api sources in repo ----" fd -HI 'mdwiki_page.py|mdwiki_page' . || true echo "---- search for save_page handling patterns ----" rg -n "if .*save_page|if .*save\b|== True|is True|newrevid" src1 -g '*.py' || true echo "---- inspect nearby caller with same pattern ----" sed -n '1,120p' src1/find_replace_bot/one_job.py echo "---- inspect another save caller ----" sed -n '1,120p' src1/md_core/mdpy/fixred.pyRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 7974
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- mdwiki_page.py outline ----" ast-grep outline src/mdwiki_api/mdwiki_page.py --view expanded || true echo "---- save() implementation region ----" rg -n "def save\(" src/mdwiki_api/mdwiki_page.py sed -n '1,260p' src/mdwiki_api/mdwiki_page.pyRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 581
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "---- inspect md_core/mdpy/imp.py save handling ----" sed -n '1,120p' src1/md_core/mdpy/imp.py echo "---- broader patterns around save result checks ----" rg -n "save_page is not True|if save_page:|if .* is True|return 1|return 0" src1 -g '*.py' || true echo "---- any docs/comments about page.save return value ----" rg -n "page\.save|save\(" README.md src1 -g '*.md' -g '*.py' || trueRepository: Mdwiki-TD/mdwiki-python-files
Length of output: 5723
Count only confirmed saves.
page.save()is already handled asTrue/not-Trueelsewhere in this codebase, soif save_page:here can treat a truthy error payload as success. Movenumbers["done"] += 1behind a strictis Truecheck so failed saves don’t consume the quota and stop the job early.🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 77-77: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(file_name[1], "a", encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').(open-filename-from-request)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src1/md_core/mdpy/replace1.py` around lines 59 - 79, Update the save handling around `page.save()` so `numbers["done"] += 1` occurs only when `save_page is True`, not merely when the result is truthy. Keep the existing revision and output-file handling aligned with confirmed saves, ensuring failed saves do not increment the quota or trigger completion logic.
102-109: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Reject an empty find string before processing pages.
Python inserts the replacement between every character when
find == "". With the default 20,000-page limit, an accidentally emptyfind.txtcan cause mass corruption.with open(find_file, "r", encoding="utf-8") as file: find = file.read() + if find == "": + raise ValueError("find.txt must not be empty")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.with open(find_file, "r", encoding="utf-8") as file: find = file.read() if find == "": raise ValueError("find.txt must not be empty") # --- with open(replace_file, "r", encoding="utf-8") as file: replace = file.read() # --- if replace.strip() == "empty": replace = ""🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 104-104: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(replace_file, "r", encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').(open-filename-from-request)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src1/md_core/mdpy/replace1.py` around lines 102 - 109, Validate the contents loaded by the find-file read before processing pages, rejecting an empty or whitespace-only find string with the existing input-error behavior. Add this guard immediately after reading find and before loading or applying replace, while preserving non-empty find values unchanged.toolforge/deploy_repo.sh (1)
97-97: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Copy hidden files and prevent globbing failures.
Using the
*wildcard will skip hidden files (e.g., dotfiles) by default. Furthermore, if$SRC_DIRis empty,*will not expand, and thecpcommand will fail with a "cannot stat" error.Use
"$SRC_DIR"/.to safely copy all contents, including hidden files, without relying on glob expansion.💻 Proposed fix
-cp -rf "$SRC_DIR/"* "$TARGET_DIR/" -v +cp -rf "$SRC_DIR"/. "$TARGET_DIR/" -v📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.cp -rf "$SRC_DIR"/. "$TARGET_DIR/" -v🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@toolforge/deploy_repo.sh` at line 97, Update the copy command in the deployment script to use the source directory’s dot path instead of the * wildcard, preserving recursive copying and the existing target while including hidden files and avoiding failures when the source directory is empty.
1 participant
Summary by CodeRabbit
New Features
Chores