[oharboe]

The stage scripts in flow/scripts/*.tcl keep to idioms that have grown up in review and were never written down. This writes them down in docs/contrib/TclContract.md as hard-stop rules, so that a contributor's AI writes stage-script code that is congruent with the existing scripts, and automated review has something to check against. The goal is more contributor autonomy: fewer review rounds spent restating the same conventions. Each rule says what breaks when it is broken.

• Idiom: use the util.tcl helpers (append_env_var, log_cmd, orfs_write_db/orfs_write_sdc) rather than writing them again; a command's arguments go through log_cmd so the log shows the full command line that a user copies into <STEP>_ARGS; procs take arguments rather than globals, and state that crosses steps lives in the .odb; never repeat an OpenROAD default; an explicit flag over a sentinel value; plain Tcl as tclint/tclfmt check it.
• Failure: never guard a read the flow guarantees (a variable with a default in variables.yaml, a declared input file); a failing step writes <step>-failed.odb and calls error, never the success artifact; no error numbers or tool checks in ORFS.
• Interface: result file names and formats are an interface and change only in their own PR; open.tcl never writes a file; a refactor never changes behavior.
• Review suggestions to decline: the suggestions automated reviewers make again and again on these scripts that break the rules above: existence guards for variables with defaults, file exists checks, error $msg $::errorInfo, prefixing RESULTS_DIR without reading the callee, "robustness" for cases the flow does not have, and untested yosys select rewrites. It also says which suggestions are worth taking.

The rules come from review threads on flow/scripts/*.tcl, among them #4563, #4543, #4416, #4365, #4619, #4688, #4685, #4418, #3680, #4168, #3076, #2700, #3330, #3610, #1652, #2100, #2236, #1596, #2610, #4486, #4212 and #4179. Existing code that breaks a rule is technical debt, not precedent.

The page links to the Makefile contract (#4689) for the step and variable rules, which apply to the stage scripts too and are not repeated. The two PRs are independent and merge without conflict; this one's link to MakefileContract.md resolves once #4689 is merged.

AGENTS.md gets one line pointing agents at the page, and the Developer's Guide and docs/toc.yml list it.

Documentation only.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces a new documentation file, TclContract.md, which outlines the rules, idioms, and guidelines for writing and reviewing stage scripts in flow/scripts/*.tcl. It also updates AGENTS.md, DeveloperGuide.md, and the table of contents (toc.yml) to reference this new contract. The review feedback focuses on improving readability by refining clunky phrasing in TclContract.md and DeveloperGuide.md, as well as ensuring formatting consistency for list items in the new contract.

[openroad-ci]

🔍 QoR check

Metrics reflect the PR merge build — i.e. what will land on the target branch.

Commit e852cff · Jenkins build #2 · Baseline: build · View build on dashboard

62 design(s) checked — 0 with regression(s), 0 without a comparable baseline.

[dsengupta0628]

LGTM as a baseline. Two small things where the page currently describes the code incorrectly, so an agent following it would make wrong changes:

1. <STEP>_ARGS "replaces the arguments the flow would pass" is true only for some steps. CTS_ARGS, RTLMP_ARGS and DETAILED_ROUTE_ARGS replace, but GLOBAL_PLACEMENT_ARGS (global_place.tcl:66), PLACE_PINS_ARGS, BUFFER_PORTS_ARGS, CLUSTER_FLOPS_ARGS and DETAIL_PLACEMENT_ARGS (util.tcl:52) append. Suggest "replaces or extends", or dropping that sentence, so an agent doesn't "fix" the appending steps into replacing ones.

2. "A script never adds error numbers" cites global_place.tcl as the example, but that file has utl::error GPL 200 (an ORFS script using OpenROAD's GPL range). ORFS also already uses its own FLW numbers (util.tcl:182, util.tcl:237). Suggest rewording to "never uses a number from an OpenROAD module's range (FLW is ORFS's)", or dropping the rule for now.

[oharboe]

@dsengupta0628 thanks, both checked against the scripts.

1. <STEP>_ARGS: fixed in the commit above. The rule now names the three that replace (CTS_ARGS, RTLMP_ARGS, DETAILED_ROUTE_ARGS) and the five that append (GLOBAL_PLACEMENT_ARGS, PLACE_PINS_ARGS, BUFFER_PORTS_ARGS, CLUSTER_FLOPS_ARGS, DETAIL_PLACEMENT_ARGS). It also says a change never switches a step from one to the other, so an agent won't "fix" the appending ones.

2. Error numbers: the rule stays, with the reasons written out. The existing numbers are the problem the rule is meant to stop:

   • IDs are OpenROAD's mechanism. Each tool owns its range, and uniqueness is the tool owner's job, checked by etc/find_messages.py (OpenROAD's docs/contrib/Logger.md). That check scans OpenROAD's src/ only; nothing checks ORFS.
   • ORFS's numbers already collide. FLW 25 is the warning place: add GPL_PLACE_IOS for global_placement -place_ios #4685 merged into util.tcl, and read binary liberty (.blib) files in the OpenROAD stages #4682 adds a different utl::warn FLW 25 "$blib not found..."; your own review there calls it "FLW-0025". Nothing caught it, and nothing in ORFS can.
   • GPL 200 sits inside GPL's own range, so it can collide with GPL itself later.
   • Most of ORFS's errors aren't OpenROAD's. They come from yosys, the memory generators, Python, Tcl and make, so a number scheme can't cover them anyway; error with a message that says what failed can.
   • A check on what a tool did belongs in the tool, which is the point of place_density_with_lb_addon mechanism should live inside GPL #4499 ("the error comes from OR and not from a script in ORFS").

   The existing FLW and GPL 200 messages aren't renumbered or removed here: the rule says they stay until a change of their own removes them, since genMetrics.py parses one (FLW-.... clock slack). The rule only stops new ones.